Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix link range in the double byte character scenario #4261

Closed
wants to merge 2 commits into from

Conversation

robinfai
Copy link

image

@Tyriar Tyriar added this to the 5.1.0 milestone Nov 30, 2022
@Tyriar Tyriar self-assigned this Nov 30, 2022
@jerch
Copy link
Member

jerch commented Dec 4, 2022

@Tyriar Is this a regression with the new linkifier? I thought this got fixed years ago by several PRs, but could have been with the old linkifier.

Edit:
I think the weblinks addon is broken in several ways, not only for wide char offsets. If I run the following script in a terminal:

#!/bin/bash

# all half width - only good case
echo "aaa http://example.com aaa http://example.com aaa"

# full width before - wrong offset
echo "¥¥¥ http://example.com aaa http://example.com aaa"

# full width between - wrong offset
echo "aaa http://example.com ¥¥¥ http://example.com aaa"

# full width before and between - error in offsets adding up
echo "¥¥¥ http://example.com ¥¥¥ http://example.com aaa"

# full width within url - partial wrong match
echo "aaa https://ko.wikipedia.org/wiki/위키백과:대문 aaa https://ko.wikipedia.org/wiki/위키백과:대문"

# full width within and before - partial wrong match + wrong offsets
echo "¥¥¥ https://ko.wikipedia.org/wiki/위키백과:대문 aaa https://ko.wikipedia.org/wiki/위키백과:대문"

# not matching at all
echo "http://test:password@example.com/some_path"

I get multiple issues at once:

  • wrong offsets for wide chars (issue at hand)
  • urls with a name portion (name@domain) dont match at all, same with name + password scheme (name:password@domain)
  • wrapped urls dont get underlined in the second line, if hovered from left or right side (weird: it works, if the pointer enters the line from top or bottom directly on the url)

At this point I am not even sure, if this all can be fixed one by one or if it needs a proper rewrite of the link matcher and the position calculations. Hmm.

Edit2:
Here is my early take on the weblinks addon issues: #4288 (WIP so far, as it surfaced a few more issues at other ends...)

@Tyriar
Copy link
Member

Tyriar commented Dec 5, 2022

@jerch yeah I feel like I've fixed this issue a few times, there are various different places it happens though, not just in links. The web links addon also hasn't got much love from me for quite a while as vscode leans primarily on monaco's link detection now.

Should this close of in favor of #4288?

@jerch
Copy link
Member

jerch commented Dec 5, 2022

Should this close of in favor of #4288?

@Tyriar Yes, I think it is not much help with all the other issues still in place. Gonna close it.

@robinfai Thx for bringing this to our attention. Sadly its not done with your changes due multiple issues at others ends. Will see if I have more success with proper url detection and underlining in #4288.

@jerch jerch closed this Dec 5, 2022
@Tyriar Tyriar removed this from the 5.1.0 milestone Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants