mini.jump (clever-f)

[xigoi]

• I have read CONTRIBUTING.md
• I have read CODE_OF_CONDUCT.md
• Works in normal mode
• Works in visual mode
• Works in operator mode
• Accepts v:count
• Internal documentation
• Highlighting

This is a module named mini.jump, implementing the basic functionality of clever-f and a basis for the functionality of vim-sneak.

Would this fit the spirit of this project? Should I also add two-char search like vim-sneak?

(Note: the implementation is currently a bit buggy, but the basic functionality works)

[echasnovski]

Wow, that is great! Thanks!

Yes, it does fit in the spirit of this project quite nicely. I don't use any of the clever jumps plugins, so it wasn't implemented earlier. I have quite a lot of questions about particular implementations, but that can wait until you feel like this is good for review. Some general tips which I try to follow:

• Use expression mappings only when they are crucially needed. Here it seems that basic mappings will be enough.
• Use string.format() instead of ... It proved to be faster and more readable.
• Export in module (MiniJump in this case) as little as possible. In this case both target and ask_for_target on the first glance seem like they should be in H.

Scanning through README of 'clever-f' it seems that highlighting possible jumps is pretty crucial to usability. Can this be implemented?

If you feel like adding two-char search which will have relatively simple and fast code, then sure, go for it. By the way, I've read many good things about ggandor/lightspeed.nvim. Maybe this should serve as a guide instead of vim-sneak?

[xigoi]

Thanks for the feedback!

I used mini.pairs as a "template" to be consistent with your coding style, which explains the expr mappings. I'll get rid of them.

Thanks for the tip about string.format, I'm quite new to vanilla Lua (I previously used MoonScript, which has built-in string interpolation).

ask_for_target has to be in the module so that it can be modified by the autocommand. Do you have an idea how to avoid this?

[xigoi]

I'll look into how to do the highlighting. Personally I find it more annoying than useful.

Lightspeed is an interesting concept, but I think it (and labeled jumps in general) might be too complex to fit with mini.

[echasnovski]

ask_for_target has to be in the module so that it can be modified by the autocommand. Do you have an idea how to avoid this?

I currently find using exported on_<autocmd>() quite convinient. So in this case it will be something like:

function MiniJump.on_cursormoved()
  H.ask_for_target = true
end

And then having autocmd CursorMoved * lua MiniJump.on_cursormoved(). NOTE: I am not entirely sure why this has to be in CursorMoved, but will do deeper dive when this is ready.

[xigoi]

I am not entirely sure why this has to be in CursorMoved

The point of clever-f is that if you press f and the last movement was f, then it will repeat the movement instead of prompting for another character. So I need some way to tell if the last movement was f. I haven't looked into how clever-f does it, but the CursorMoved trick seems to work well.

[xigoi]

It looks like search() has no way to get positions of all matches, so highlighting would need to be done by manually scanning the buffer, which could be quite complex and error-prone.

[xigoi]

I think it's pretty much ready for review. It's just missing documentation in README and it's unclear how t should work when the match is at the beginning of a line (currently I just skip such matches, but that's probably a bad approach).

[echasnovski]

Thank you for this! I'll take a closer look along with hands-on testing this week.

What I can say right now:

• I think that the main point of this plugin is to enhance behavior of jumping. So having it do nothing by default is a bit confusing. Currently I think that in terms of mappings it should be similar to 'mini.surround': have a config.mappings table defining what mappings are made. And all those mappings are applied by default.
• Right now I am in the middle of figuring out a better way of introducing new modules. This will probably have a public beta-testing period inside other directory (something like 'lua/mini-dev'). I plan to soon add two modules there (almost finished). This one will go with them, once I decide on process.
• I encourage you to add yourself as second copyright holder (without any obligatory burden of supporting this in the future). If you don't want that, at least mention yourself in module's documentation.
• External documentation is generated locally on my machine. It is currently using tjdevries/tree-sitter-lua which is a bit of a pain to setup. I'll generate it myself after it is merged.
• Having highlighting would be super cool (without complicated code, of course). Maybe look at how 'clever-f' does this?

[xigoi]

Here is the relevant function from clever-f:

function! clever_f#_mark_direct(forward, count) abort
    let line = getline('.')
    let [_, l, c, _] = getpos('.')

    if (a:forward && c >= len(line)) || (!a:forward && c == 1)
        " there is no matching characters
        return []
    endif

    if g:clever_f_ignore_case
        let line = tolower(line)
    endif

    let char_count = {}
    let matches = []
    let indices = a:forward ? range(c, len(line) - 1, 1) : range(c - 2, 0, -1)
    for i in indices
        let ch = line[i]
        " only matches to ASCII
        if ch !~# '^[\x00-\x7F]$' | continue | endif
        let ch_lower = tolower(ch)

        let char_count[ch] = get(char_count, ch, 0) + 1
        if g:clever_f_smart_case && ch =~# '\u'
            " uppercase characters are doubly counted
            let char_count[ch_lower] = get(char_count, ch_lower, 0) + 1
        endif

        if char_count[ch] == a:count ||
            \ (g:clever_f_smart_case && char_count[ch_lower] == a:count)
            " NOTE: should not use `matchaddpos(group, [...position])`,
            " because the maximum number of position is 8
            let m = matchaddpos('CleverFDirect', [[l, i + 1]])
            call add(matches, m)
        endif
    endfor
    return matches
endfunction

It seems to find and highlight the matches manually, which could be very error-prone, especially with Unicode characters. Also it works only on the current line.

I just got an idea on how I might be able to (ab)use syntax highlighting to do it.

[xigoi]

Wow, that was actually very easy! Now the question is, should we highlight matches after/before the cursor, or all of them? The former may or may not be tricky.

[xigoi]

Just curious, what modules are you adding?

[echasnovski]

I believe, having highlighting only at those places that are reachable with current command (fFtT) is more appropriate. This can be solved by either more elaborate highlighting or having commands cycle from end to beginning.

I just quickly tried this module and here are some first thoughts:

• It definitely needs more careful handling of highlighting and state tracking. It doesn't disappear when going in Insert mode. Also there is some weird freeze when going into Telescope while highlighting is visible. Probably, reset_target() should be called also on InsertEnter and BufLeavePre.
• Right now I can see it being useful for quick jumping to single letter, allowing till and automatic disappear of highlighting. Otherwise it seems to be quite similar to / and nN. Should think about needing multiple character support.

[echasnovski]

Just curious, what modules are you adding?

https://github.com/echasnovski/nvim/tree/master/lua/mini-dev . That is my alpha-testing :).

[xigoi]

Right now I can see it being useful for quick jumping to single letter, allowing till and automatic disappear of highlighting. Otherwise it seems to be quite similar to / and nN. Should think about needing multiple character support.

My intent was to improve the built-in f and t commands without adding anything new. I think 2-char search should be opt-in, but I can add a way to define mappings for it. And when it comes to / and ?, it might be a good idea to somehow hijack them too.

[echasnovski]

Just wanted to say about the problem with multiple consecutive matches :)
I had quite a number of small changes (mostly cosmetic). Do you mind me pushing changes here?

[xigoi]

No, it's your plugin after all :)

[echasnovski]

Most of the changes are cosmetic. Ones that have side effects are in commit's details.

Current thoughts and problems I found so far:

• I am not sure if target should or should not be reset when using from different modes. So say I used it from operator-pending mode with dt). Should I manually supply target if I press t right after that?
• Highlighting definitely needs more attention. There were some moments when it didn't disappear. Not sure right now what caused them. Edit Oh, at least one of situation is when testing this issue with macros. Which is not completely absent here either by the way.
• I am currently getting more and more inclined to make this just an enhancement for FfTt (as you originally intended). This might get changed if we'll come up with a more user-friendly way to support multiple characters. If only FfTt-like functionality will be supported, changing a name that reflects that might be a good idea (although I really like the 'mini.jump').
• There is got to be a better way to implement smart jumping and its resetting than current one. I don't have a good feeling about using CursorMoved and vim.schedule (inside smart_jump).

[xigoi]

• I didn't consider this. I think it should be reset.
• The issue in clever-f links to an issue in lightspeed, which seems to be caused by feedkeys(). I don't use feedkeys() though...
• It wouldn't be a problem to allow mapping something to 2-char search the same way that f is mapped to 1-char search.
• It looks like clever-f uses CursorMoved. But if you can come up with something better, feel free to suggest it.

[echasnovski]

Think I solved most of the immediate issues I found while using it. It seems that most of them were because of vim.schedule() might execute its input too late. Also matchadd() works per window, so I adapted 'mini.cursorword's code here with slight change in logic of unhighlight().

I currently kind of like that after dt) I can still use jumping.

Another good idea might be to allow repeating current jump and encourage to use it with ; (via config.mappings.repeat with '' as default). This might be a better "long-term" behavior (don't need to retrain oneself from using ;) and easier for switching.

I plan to test it through everyday usage and see if there is something we missed. Adam, could you, please, do the same?

[xigoi]

I'm definitely using the module (I have my fork installed as a plugin) and it seems fine for me. But then, I don't use Telescope, so I can't find Telescope-related bugs.

The ; is a good idea for a transition period. I wouldn't encourage it because the main advantage of clever-f is freeing up the ; and , keys. (I personally have ; mapped to <cmd>w<cr> and love it.)

[echasnovski]

@xigoi, I've decided to make a quite big refactor. Didn't do it in this PR to not break your setup. Would you mind looking at this commit in my neovim config and tell me your thoughts?

After reaching an agreement regarding these changes I'll plan to merge this PR as is, move it to 'mini-dev' instead of 'mini', make those new changes, and it will be ready for a public beta-testing (hopefully in the upcoming week).

[echasnovski]

@xigoi, I'll merge this as is and make updates after. I recently after a lot of trial, error, and reading Vimscript code managed to implement fool dot-repeat in operator-pending mode. Also made a refactor to easy allow MiniJump.repeat(), but I'll wait and see about it.

Thank you very much for your ideas, enthusiasm and work!