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

Ability to update prefills with visual selection #258

Closed
mehalter opened this issue Sep 19, 2024 · 9 comments · Fixed by #263
Closed

Ability to update prefills with visual selection #258

mehalter opened this issue Sep 19, 2024 · 9 comments · Fixed by #263

Comments

@mehalter
Copy link
Contributor

I love the improvements to the API and particularly enjoy the ability to update the prefills of a given instance. Given that the prefilling with visual selection is a separate command, could there be an addition to the API to allow updating the prefills with the current visual selection as well?

@MagicDuck
Copy link
Owner

hi @mehalter,

So the way visual selection currently works is a bit special in that depending on the engine (ripgrep/astgrep) we put in additional flags. For example, for ripgrep, we add --multiline --fixed-strings. This works on initial launch but would feel weird on an update_instance_prefills() call.

I think what we can do though and would be the most flexible is expose a utility function to get the current visual selection as a string, that users can then pass to update_instance_prefills(). Seems like a common enough thing and would be annoying to rewrite.

Implementation would look something like:

function M.get_current_visual_selection()
  local isVisualMode = vim.fn.mode():lower():find('v') ~= nil
  if isVisualMode then
    -- needed to make visual selection work
    vim.cmd([[normal! vv]])
  end

  local selection_lines = utils.getVisualSelectionLines()
  return vim.fn.join(selection_lines, '\n')
end

So you would call it like:

grugFar.update_instance_prefills('myinstance', { search = grugFar.get_current_visual_selection() })

The beauty of this is that you can modify it before passing it in or prefill some other input like replace.

@mehalter
Copy link
Contributor Author

Oh yeah that looks great!

@MagicDuck
Copy link
Owner

done 😄

@MagicDuck
Copy link
Owner

let me know if you see any issue, it was sort of hastily thrown in 😄

@mehalter
Copy link
Contributor Author

I'll let you know tomorrow when I mess around with it. Only thing that comes immediatly to mind is it seems like there might be a good way to interpolate this through the open and with_visual_selection functions to save on duplicate code

@MagicDuck
Copy link
Owner

Those ones already use the util function, and are slightly different since they need to call on engine specific logic, but if there is a nice way to do it I am not against it…

@mehalter
Copy link
Contributor Author

@MagicDuck I took a stab at this while also fixing a few bugs in the visual selection code. Mainly bugs in how the code was leaving visual mode which was exposed when using the update_prefills method. And another bug that was breaking visual line mode. I opened up a PR that should be good for review now. This refactor did lead to a nice way to centralize the semantics of "getting a visual selection" so that it is retrieved the same way across the code base. Hope this helps! I'm glad to get to contribute to a great project :D

PR #265

@MagicDuck
Copy link
Owner

Thank you for those, I wish I could get more people like you, who create PRs instead of just asking, haha!

@mehalter
Copy link
Contributor Author

No problem! Team work makes the dream work in the open-source community 🤗

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 a pull request may close this issue.

2 participants