Opened 2 months ago

Last modified 2 months ago

#18820 new bug

[Terminal] HyperLink mode fails for paths with escaped chars.

Reported by: bipolar Owned by: jackburton
Priority: normal Milestone: Unscheduled
Component: Applications/Terminal Version: R1/beta4
Keywords: Cc:
Blocked By: Blocking:
Platform: All

Description

If you have paths such as /foo-[bar]/baz, or /foo/foo~bar/baz, when those get listed in Terminal (as results of a query call for example), they fail to be recognized as "path" hyperlinks when you hover over them while holding CMD down.

They get listed as:

/foo-\[bar\]/baz
/foo/foo\~bar/baz

And it seems to me that it is that "\" escaping what it is making the "hyperlinking" fail.

---

Tried to fix it myself, but failed miserably. Halp? :-)

Change History (2)

comment:1 by pulkomandy, 2 months ago

The function to detect links is _GetHyperlinkAt:

https://cgit.haiku-os.org/haiku/tree/src/apps/terminal/TermViewStates.cpp?id=52100b0c0e79c475267b510420f1f1da133005ca&h=master#n750

First of all, this function will use DefaultCharClassifier to try to find a block of text that could be a path or URL. This uses a simple algorithm to split the text into pieces:

  • Spaces are handled as a special case, they may be part of a path, but are also a good place to look for a path start or end
  • Alphanumeric chars are likely to be inside a path
  • Additionally, a few special characters are also likely to be inside a path. These are currently defined in TermConst.cpp, kDefaultAdditionalWordCharacters and kURLAdditionalWordCharacters

These list do not include \. So, paths with a \ will not be matched. The solution to that is simple: add it to the list there. It seems you also want to add [ and ]. The risk with adding too much things is that, at some point, Terminal will think that everything is a path.

Once this is done, the paths should start highlighting. Another step will probably be to unescape these characters to remove them from the string. I think BString::CharacterDeescape can be used for this. This should be done before passing the string to _EntryExists (which checks if the path points to an existing file, to avoid false positives) or maybe inside _EntryExists (so it can try the path with and without de-escaping).

comment:2 by bipolar, 2 months ago

Thanks a bunch Adrien.

I *had* tried adding "\\[]", but still wasn't getting proper matches on _GetHyperlinkAt.

After closer inspection (and some coffee, and sweat, and tears :-D):

Seems that introducing a classifier for absolute paths (instead of trying to re-use fCharClassifier :-D), and doing that CharacterDeescape() before _EntryExists, did the trick!

Will do more testing/clean up, and submit a patch for review.

Thanks again!

Last edited 2 months ago by bipolar (previous) (diff)
Note: See TracTickets for help on using tickets.