Weyaaron / nvim-training

A plugin to practice neovim movements, currently in "open beta"
GNU General Public License v3.0
36 stars 3 forks source link

feat: YankWord task #11

Closed mertovun closed 5 months ago

mertovun commented 5 months ago
Weyaaron commented 5 months ago

I think some of the additions to the utility class are neat and will be merged eventually I suppose. But the way your new method 'create highligt'(See code comment on this pr) works is not good coding policy. A function should do one thing and one thing only. A function that creates a highlight should not do the unrelated side effect of returning something unrelated to the highlight.

My suggestion is that I leave this open and maybe my suggestions could be implemented on top? If not, you might be better off splitting this pr into the utility and the task part. I have no particular preference.

Weyaaron commented 5 months ago

And I am unsure if the movement belongs to the task at all. Maybe its best to stick to the word at the cursor instead of requiring some additional steps.

mertovun commented 5 months ago

And I am unsure if the movement belongs to the task at all. Maybe its best to stick to the word at the cursor instead of requiring some additional steps.

I see your point. The extension could offer both granular tasks for individual commands and composite tasks for more realistic editing scenarios. I believe navigating to a word and copy it is a fundamental operation in the daily use and lies somewhere in between. We could consider renaming the current task to better reflect its intermediate nature, to address your concerns.

Weyaaron commented 5 months ago

I have opened an issue on this, we should discuss it over there. I will merge this particular instance and go from there, I appreciate the contribution and the discussion.

Weyaaron commented 5 months ago

And I will take care of the renaming and might make some slight adjustments for the pr into main.

Weyaaron commented 5 months ago

There is some code left over that does not align with the desired changes, I will do this as well, I consider this a minor issue.

Weyaaron commented 5 months ago

There is some code left over that does not align with the desired changes, I will do this as well, I consider this a minor issue.

These turned out to be not that minor, but it helped me with interacting with your code. All should be done now, if you have concerns let me know.