hadronized / hop.nvim

Neovim motions on speed!
Other
2.48k stars 138 forks source link

Consistent jump hints #198

Closed mikeslattery closed 2 years ago

mikeslattery commented 2 years ago

When I use hop, the jump hints seem randomly ordered. It would be nice if the first, second, third, forth, etc positions were always the same character hints (respectively as asdf), like in EasyMotion.

For example, if I had f mapped, and s always hinted the 2nd occurance, and I wanted to go to the 2nd occurance of x I might type: fxs

Hop's random ordering slows me down compared to EasyMotion as I have to wait and read the hints before I jump, and I can't use them in macros.

Chaitanyabsprip commented 2 years ago

from what I have experienced and even read about the algorithm that phaazon uses, the second occurence will always be marked with the same key. https://phaazon.net/blog/blog/hop-trie-backtrack-filling.

hadronized commented 2 years ago

The article is https://phaazon.net/blog/blog/hop-trie-backtrack-filling. Order is not enforced but will most of the time be an emergent property.

bew commented 2 years ago

The article is https://phaazon.net/blog/blog/hop-trie-backtrack-filling. Order is not enforced but will most of the time be an emergent property.

Note that this URL gives a 404, removing one blog/ fixes it, but the page seems to rewrite the URL to add the second blog/ :eyes:

mikeslattery commented 2 years ago

Interesting and helpful article, thank you. Tbh, I'd prefer predictability and consistency over optimal permutation length. I want to type fast without halting to look at nearby hints that I can easily predict.

I'll experiment with restoring the original perm_method algorithm (TermSeqBias) for 1-char searches (after switching back to Hop from EasyMotion) and I'll post my thoughts about it here when I'm done. Although it's mentioned in the docs, TermSeqBias is no longer implemented in the source.

mikeslattery commented 2 years ago

My experiment failed to reliably give hints (in asdfghkl... order) when using the original v0.0.1 algorithm. Turns out, the core of the problem is that hint.lua implements the 2D Manhattan Distance instead of simply using the 1D/linear file offset as the distance.

A poor workaround is to set current_line_only = true, so the 2D distance algo can't take row distance into account.

This may be too big to fix in Hop without a major refactor. I'm going back to EasyMotion until either someone writes a PR for this issue, or I find the time to rewrite portions of hint.lua.

JoannesJ commented 2 years ago

Quick solution would be to crank up the x_bias on the manh_dist function to a number bigger than any practical line size. Beyond that, I'm not sure if there is any meaningful consistency to be gained.

mikeslattery commented 2 years ago

Closing. I've decided to no longer use f/t/F/T mappings of hop. s is king. Plus, no one else seems interested.

mikeslattery commented 1 year ago

FYI, I came to the realization that Hop's design focus doesn't match my goals.

Hop is an EasyMotion-like plugin allowing you to jump anywhere in a document with as few keystrokes as possible

Fewest keystrokes is not a goal, it's a strategy. I want speed, which for me means least cognitive load. I want to move without thinking. Paying attention to seemingly randomized hints is antithetical to that goal.

Chaitanyabsprip commented 1 year ago

what plugin have you replaced hop with? I am interested in your idea of a jump plugin, prioritizing cognitive load over keystrokes makes sense to me.

yanshay commented 1 year ago

Quick solution would be to crank up the x_bias on the manh_dist function to a number bigger than any practical line size. Beyond that, I'm not sure if there is any meaningful consistency to be gained.

@JoannesJ I tried your suggestion but it didn't work. Changing the x_bias does have an effect but it doesn't generate consistency even on the current line.

I understand there are some optimizations that drive the inconsistency. I think a good tradeoff could be consistency for current line and then same optimizations for other lines.

JoannesJ commented 1 year ago

@yanshay Did you do it with the direction option set?

yanshay commented 1 year ago

@yanshay Did you do it with the direction option set?

@JoannesJ yes, and as said, it does have an effect but it doesn't maintain consistency even on the current line no matter the number I use.

mikeslattery commented 1 year ago

@Chaitanyabsprip Leap

You use the actual 2 characters at the location, with an optional 3rd char for secondary location hints. The 3rd character is not the same character at the location, but it's consistent (e.g. the 4th location is always j for me). What's nice is you see the 3rd character hint after typing the 1st, so it gives your brain a few milliseconds to process it.

It's not nearly as keystroke efficient as Hop. I'd say it takes one extra keystroke over Hop on average.

Chaitanyabsprip commented 1 year ago

I have also been using leap, and it takes minimum 3 keystrokes to get anywhere, maybe if leap adopts hop's algo or hop implements the eager hinting then we'll have our sweetspot

yanshay commented 1 year ago

@JoannesJ I think I found a solution by looking at hints generated when I changed the bias:

-- Manhattan distance with column and row, weighted on x so that results are more packed on y.
function M.manh_dist(a, b, x_bias)
  local bias = x_bias or 1000
  return bias * math.abs(b[1] - a[1] + 1) + math.abs(b[2] - a[2])
end

It seems to be there's a bug in the calculation. I noticed that the first keys are always on the line right after where the cursor is positioned, unless there are no matches on that (next) line. So I added 1 to the subtraction operation and it now use the first keys for the current line. Add to that your suggestion to increase the bias and I receive now consistent hints. I wonder what would be the proper way to expose this functionality for consistent results.

I would actually make the distance such that current line gives lowest distance on all targets and on other lines maybe behave the same as before. Something like this:

-- Manhattan distance with column and row, weighted on x so that results are more packed on y.
function M.manh_dist(a, b, x_bias)
  local bias = x_bias or 10
  local base_dist = 1000
  if (b[1]-a[1] + 1 == 0) then
        base_dist = 0
  end
  return base_dist + bias * math.abs(b[1] - a[1] + 1) + math.abs(b[2] - a[2])
end

Thoughts?

ggandor commented 1 year ago

I have also been using leap, and it takes minimum 3 keystrokes to get anywhere, maybe if leap adopts hop's algo or hop implements the eager hinting then we'll have our sweetspot

@Chaitanyabsprip "eager hinting", by design, is only possible for a 2-character pattern (what to show if there's no input yet?), so it's either 2 keystrokes with pause in-between, or 3 keystrokes without pause, there's no "better" way.

if leap adopts hop's algo

What to adopt? If the search is bidirectional, the matches are ordered by distance from cursor, just like in Hop. And for default s/S, the matches are labeled in the same order as they are in the buffer (obviously).

JoannesJ commented 1 year ago

@yanshay

That does seem like a bug. It probably has something to do with the offset on window_context.top_line. But I don't remember why that was there.

I wonder what would be the proper way to expose this functionality for consistent results.

I don't use Hop anymore, so don't have a strong opinion. Exposing x_bias, or hard-coding a current line bias; they both seem fine.

hadronized commented 1 year ago

I think adding another way of labelling could be done by removing the « sort by distance to cursor » and instead « sort by distance to the top-left corner of the buffer view ». That should be fairly simple to do, it’s just not there.

yanshay commented 1 year ago

@phaazon The issue is about consistent hints. The way I see it mostly about - if I know I need the first or second or third occurance of a target, I know which hint letter to press before I even start typing. So for example, if I want to reach the second 'i', I know to press 'is' (assuming keys are as 'asdghk...') . I believe this requirement is usually relevant mostly for the current line, maybe for nearby lines as well.

If distance would be from top-left corner of buffer, then it would also be inconsistent from that respect, isn't that the case? On every line the second 'i' after the cursor would have a different hint.

So the solution IMO is first a fix to the bug above, not sure if the fix I suggested (simply adding 1 to the subtraction) is correct because I don't know what drives the incorrect calculation, maybe it depends on other configurations that cause the offset.

And then on top of that, the distance for nearby lines should take precedence over the standard distance. But to get consistency it needs to be distance from cursor. I implemented such modification for current line above as well.

Or maybe, even better, distance could be to prioritize the first closest occurrences that are in continuation to the text (to the extent a person can see in advance, let's say up to 5) and after that go back to standard distant calculation (which I guess, there's a reason to having it using the current algorithm). Such solution seems more complex to achieve than the first option.

ggandor commented 1 year ago

Frankly, I don't understand the point of this whole long discussion here. If the search is bidirectional, then whatever you do, the hints cannot be made predictable - you either want to go forward or backward, but there can be matches in both directions, and Hop cannot read your thoughts. And if the search is directional (AC/BC), then where's the problem (just keep their original order, as they follow each other in the buffer)?

yanshay commented 1 year ago

And if the search is directional (AC/BC), then where's the problem (just keep their original order, as they follow each other in the buffer)?

This is the relevant case, and they don't follow each other in current implementation. It's based on a distance calculation and a match in the row below could be closer than a match in current row. So it's unpredictable and you can't use Hop for macros because of that, which I see as a major limitation. To create a macro I need to disable hop, record/replay a macro with f/F, use it and then re-enable Hop.

ggandor commented 1 year ago

And the solution is trivial then, just add a conditional check which skips the unnecessary sorting step for AC/BC motions.