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

feat: allow users to supply custom distance functions #66 #67

Merged
merged 2 commits into from
Apr 20, 2024

Conversation

johnlevidy
Copy link

@johnlevidy johnlevidy commented Mar 14, 2024

I explored different options, but ultimately it seems sorting the distance is the only way to get consistent read-wise ordering. The custom function doesn't take xbias, since users can supply that themselves, and win_bias is borrowed from elsewhere, since window params are not ( yet? ) provided to the custom function.

Tested with this config:

local hop = require('hop')

local readwise_distance = function(a, b)
        return (100 * math.abs(b.row - a.row)) + (b.col - a.col);
end

hop.setup{keys = 'asdfjklqweruiopzxmtgbyhn', x_bias = 10,
          custom_distance_fn = readwise_distance }

local directions = require('hop.hint').HintDirection
vim.keymap.set({'o', 'n'}, 'f', function()
  hop.hint_char1({ direction = directions.AFTER_CURSOR, current_line_only = false})
end, {remap = true})
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab
cccccccccccccbccccccccccccccccccccccc

Without the custom function, if you go above the "b" on the second line and hit "fb", the b on the second line is designated character a.

@@ -10,6 +10,7 @@ M.quit_key = '<Esc>'
M.perm_method = require('hop.perm').TrieBacktrackFilling
M.reverse_distribution = false
M.x_bias = 10
M.custom_distance_fn = nil
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
M.custom_distance_fn = nil
M.distance_method = nil

also change the default from nil to manh_dist.

@@ -114,9 +114,15 @@ local function create_line_indirect_jump_targets(jump_ctx, locations, opts)
for _, jump_target in pairs(line_jump_targets) do
locations.jump_targets[#locations.jump_targets + 1] = jump_target

local score = nil
if opts.custom_distance_fn ~= nil then
score = opts.custom_distance_fn(jump_ctx.win_ctx.cursor, jump_target.cursor) + win_bias
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor the distance method to have the same signature as manh_dist.

Copy link
Author

@johnlevidy johnlevidy Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EDIT: had written that people would have no need for x_bias, given they're already defining a custom fn, but keeping the signatures the same is a good idea. updated.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to place manh_distance in a module that's available already, if that's not a good place please lmk. felt reasonable since users will regularly require hint for all sorts of other things

@@ -114,9 +105,11 @@ local function create_line_indirect_jump_targets(jump_ctx, locations, opts)
for _, jump_target in pairs(line_jump_targets) do
locations.jump_targets[#locations.jump_targets + 1] = jump_target

local score = nil
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will remove this and inline the function call

@johnlevidy
Copy link
Author

Sorry about the long delay, had a HW issue on my linux box and just got it back up. I think the PR should be in good shape now, please let me know if you've got any further feedback!

@smoka7
Copy link
Owner

smoka7 commented Apr 14, 2024

Don't worry about the delay. I'll just have to edit some docs before merging. Thanks for the PR.

@smoka7 smoka7 merged commit 1b355b0 into smoka7:master Apr 20, 2024
2 checks passed
@johnlevidy johnlevidy deleted the custom_distance branch June 8, 2024 18:35
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.

2 participants