MentionFilter base_url config question

[JuanitoFatas]

Hi. I am using MentionFilter, and my user lives in www.lvh.me:3000/~jch.

HTML::Pipeline.new [
  HTML::Pipeline::MarkdownFilter,
  HTML::Pipeline::SanitizationFilter,
  HTML::Pipeline::MentionFilter
], context.merge(gfm: true, base_url: '/~')

If I specified base_url: '~' or /~, it gives me

www.lvh.me:3000/~/jch

instead of

www.lvh.me:3000/~jch.

How to achieve behaviour as mentioned with MentionFilter?

Currently I replace it by myself:

text.gsub!(/@([a-z0-9][a-z0-9-]*)/i) do |match|
  %Q(<a href="/~#{$1}">#{match}</a>)
end

Thanks!

[simeonwillbanks]

Quickly looking, File.join(base_url, login) is probably the culprit.

> base_url = '/~'
"/~"
> login = 'jch'
"jch"
> File.join(base_url, login)
"/~/jch"
> base_url = '~'
"~"
> File.join(base_url, login)
"~/jch"

Maybe we stop using File.join? It's been discussed in the past.

[jch]

Hmm, ya URI.join looks more appropriate for that case, though it would fix @JuanitoFatas's problem. Also, URI.join does not allow 2 relative paths:

irb(main):003:0> URI.join('/~', "jch")
URI::BadURIError: both URI are relative

Replacing this with a regular string concatenation base_url + login also won't work because users may or may not include a trailing slash on base_url:

irb(main):006:0> File.join("https://foo.com/", "jch")
=> "https://foo.com/jch"
irb(main):007:0> File.join("https://foo.com", "jch")
=> "https://foo.com/jch"

We could check whether base_url includes a trailing slash before we concatenate...

[JuanitoFatas]

I wrote two tests, now just need to figure out the code... 😓

  def test_base_url_tilde_with_slash
    body = "<p>Hi, @jch!</p>"
    link = "<a href=\"~jch\" class=\"user-mention\">@jch</a>"
    assert_equal "<p>Hi, #{link}!</p>",
      filter(body, '/~').to_html
  end

  def test_base_url_tilde_without_slash
    body = "<p>Hi, @jch!</p>"
    link = "<a href=\"~jch\" class=\"user-mention\">@jch</a>"
    assert_equal "<p>Hi, #{link}!</p>",
      filter(body, '~').to_html
  end

[simeonwillbanks]

Conditionally concatenating like @jch mentioned:

> login = "jch"
"jch"
> base_url = "https://foo.com/"
"https://foo.com/"
> base_url << "/" unless base_url[-1..-1] =~ /\/|~/
nil
> base_url << login
"https://foo.com/jch"
> base_url = "/"
"/"
> base_url << "/" unless base_url[-1..-1] =~ /\/|~/
nil
> base_url << login
"/jch"
> base_url = "~"
"~"
> base_url << "/" unless base_url[-1..-1] =~ /\/|~/
nil
> base_url << login
"~jch"

Should we add more conditions to the Regexp? Overall, seem reasonable?