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

Unexpected highlight #14

Open
walles opened this issue Dec 31, 2020 · 4 comments
Open

Unexpected highlight #14

walles opened this issue Dec 31, 2020 · 4 comments
Labels
upstream Changes required in some dependency

Comments

@walles
Copy link
Owner

walles commented Dec 31, 2020

The Problem

black is highlighted even though I think it shouldn't. Either explain this or fix it!

why-is-black-highlighted

The diff

--- /tmp/pirate/pirate-ipsum-before.txt	2020-12-31 00:11:15.000000000 +0100
+++ /tmp/pirate/pirate-ipsum-after.txt	2020-12-31 00:11:46.000000000 +0100
@@ -1,6 +1,7 @@
-Hornswaggle knave coffer rum Nelsons folly bilge water lugger. Fire in the hole black
-spot knave come about jury mast coxswain rutters. Keelhaul hail-shot Jack Ketch no prey,
-no pay gunwalls gaff haul wind. Ho fire in the hole Sail ho booty rum trysail hail-shot.
-Knave Letter of Marque barkadeer league mizzen strike colors spike. Jack Ketch spirits
-hail-shot long clothes walk the plank gabion warp. Poop deck holystone black spot tackle
-long boat loot run a shot across the bow.
+Hornswaggle knave coffer rum Nelsons folly bilge water lugger. Fire in the hole
+black spot knave come about jury mast coxswain rutters. Keelhaul hail-shot Jack
+Ketch no prey, no pay gunwalls gaff haul wind. Ho fire in the hole Sail ho booty
+rum trysail hail-shot. Knave Letter of Marque barkadeer league mizzen strike
+colors spike. Jack Ketch spirits hail-shot long clothes walk the plank gabion
+warp. Poop deck holystone black spot tackle long boat loot run a shot across the
+bow.
@walles
Copy link
Owner Author

walles commented Jan 1, 2021

git bisect blames 6243516:

6243516b7de4ba19157cb8a5ed4988539b0d339d is the first bad commit
commit 6243516b7de4ba19157cb8a5ed4988539b0d339d
Author: Johan Walles <johan.walles@gmail.com>
Date:   Wed Nov 11 23:46:04 2020 +0100

    Refine by word rather than by character

    Hello #7.

 README.md      | 10 ++++++----
 src/refiner.rs | 36 +++++++++++++++++++++---------------
 2 files changed, 27 insertions(+), 19 deletions(-)

@walles
Copy link
Owner Author

walles commented Jan 1, 2021

Smaller test case:

--- /tmp/pirate/pirate-ipsum-before.txt	2020-12-31 00:11:15.000000000 +0100
+++ /tmp/pirate/pirate-ipsum-after.txt	2020-12-31 00:11:46.000000000 +0100
@@ -1,6 +1,7 @@
-the hole black
-spot
+the hole
+black spot

@walles
Copy link
Owner Author

walles commented Jan 2, 2021

Explanation

  old: the _ hole  _ black \n spot \n
johan:             ^       ^^
 riff:             ^ ^^^^^

  new: the _ hole \n black  _ spot \n
johan:            ^^        ^
 riff:               ^^^^^  ^

So I see this as a space and a newline switched places.

And riff thinks that in this case, black (note leading space) has been removed from the first line, and black (note trailing space) has been added to the second line.

Given that riff works with words rather than characters (see #7), riff's solution contains just as many changes as mine, and both are valid.

I still don't like it though.

@walles
Copy link
Owner Author

walles commented Jan 2, 2021

I think the correct solution to this would be to say that highlighting words is more expensive than highlighting non-words.

But that would require support in https://github.com/distil/diffus which is not there right now.

walles added a commit that referenced this issue Jan 2, 2021
@walles walles added the upstream Changes required in some dependency label Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Changes required in some dependency
Projects
None yet
Development

No branches or pull requests

1 participant