Skip to content

feat(linewise-textobjs): add count and iteration support #112

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheSast
Copy link
Contributor

@TheSast TheSast commented Nov 19, 2024

It should be noted this does not yet add support for the direction in which iteration goes, thus, right now it always goes down-wards for non-block textobjs.
I'll normalize variable names and update documentation later on.
I've already been running these changes daily for more than a month, slowly ironing out any issues I found, but I figured it would be important to set up a draft pr for the purpose of discussion, especially around the indentation textobjs.

Checklist

  • Used only camelCase variable names.
  • If functionality is added or modified, also made respective changes to the
    README.md (the .txt file is auto-generated and does not need to be modified).

It should be noted this does not yet add support for the direction in which iteration goes.
@chrisgrieser
Copy link
Owner

chrisgrieser commented Nov 19, 2024

Thanks for the effort, but tbh, I am not entirely sure if I want this merged.

Iteration/counts are a relatively niche feature most people won't use. Instead, I (and probably most people) simply go to a line with the desired indentation level before using ii/ai, which gives the same result, but in a simpler way.

Thus, it's my impression that this PR falls into the category of "increase complexity significantly for a niche use case for a minority of users," something I generally learned to avoid when maintaining OSS projects.

@TheSast
Copy link
Contributor Author

TheSast commented Nov 23, 2024

I understand where you are coming from.
Since I was already planning to add count support to the charwise textobjs, as I find their lack of count support a dealbreaker, what would you suggest me to do? should I fork this project directly or set up a new plugin containing only the subset of textobjs I use and consequently can properly maintain?

@chrisgrieser
Copy link
Owner

chrisgrieser commented Nov 23, 2024

Depends, really. There are various options:

  • You make an "extension" for various-textobjects, which hooks into the plugin and modifies the behavior, making this plugin basically a dependency of yours
  • you create a fork / separate plugin
  • in case the changes to add count support are not that big, and you can explain to me why it's a dealbreaker there (I honestly have no idea, since I don't use counts normally), we could also consider merging them here

Depends a bit on your preference and the concrete situation

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