kylechui / nvim-surround

Add/change/delete surrounding delimiter pairs with ease. Written with :heart: in Lua.
MIT License
2.98k stars 60 forks source link

Clean up the codebase #157

Closed kylechui closed 1 month ago

kylechui commented 1 year ago

Use techniques/strategies in Martin's Clean Code and Fowler's Refactoring to pay off some technical debt and organize the code in a more intuitive, easier-to-understand fashion.

fnune commented 1 year ago

Hi there! At risk of being a total snoop, here's a self-serving suggestion:

I'm writing a Markdown plugin to add links from a visual selection, or a motion, or the word under the cursor. This is the goal functionality:

My new note is about storage.
               ^ (press glt.)
My new note is [about storage](about-storage.md).

My new note is about storage.
   ^ (press gl2e)
My [new note](new-note.md) is about storage.

I realized that a lot of the code I need to write is basically very similar to tpope's (and also your) surround plugin, so I thought of checking it out. As it turns out, a lot of what I wanted to implement is already implemented in your plugin.

This is part of my TODO list for my plugin:

-- -- Adding a link --
-- Get some input text, e.g. "this is what I want to link", which may be multi-line
-- Get the cursor position
-- Iterate over file incrementally until all the input has been found
-- Record the starting and ending positions of the input in the buffer

That's exactly what your code does, e.g. in queries.lua, patterns.lua and and motions.lua.

I wonder if one refactoring goal for your plugin could be to remove this functionality into its own library. Then your code would become cleaner, and I would be able to use it :)

I'm open to helping out if you think this is an interesting idea. Otherwise, I can always copy the code and attribute, if you're OK with that.

kylechui commented 1 year ago

Hi there! Just to be upfront about this idea: I have entertained the notion of abstracting a few of the modules in nvim-surround into their own repository, but remain hesitant because I would need to:

While I'm not opposed to the idea in the long run, I think that it isn't something I'm going to focus on at the moment. I'm still working on even getting a working version of queries.lua, and would like to fix a lot of the code/make it more understandable before i distribute it as a "just works" library for others.

As for your plugin idea in particular:

Feel free to tag me if you have any more questions/comments!

fnune commented 1 year ago

Interesting!

  • Create a new repository which nvim-surround depends on
    • This introduces breaking changes for all of my users downstream
  • I would need to manage complexity with respect to what should be "intended" behavior for any kind of new motion/text-object, and coordinate with downstream plugin developers, etc.

As for (1), I imagine the library can be hosted in the same repository and also distributed via LuaRocks. But as for (2), yeah, that can't be dodged :)

I think for now I'll vendor some of your code and attribute back to you. Still, I don't know if I'll ever publish the plugin, but we'll see :)

Thank you by the way! And sorry for hijacking the issue!

kylechui commented 1 year ago

The idea of distributing via LuaRocks seems interesting; I've never experimented with that before and I don't know how that ties into Neovim. Thanks for the tip though!

fnune commented 1 year ago

It came from the unchecked assumption that one can use luarocks dependencies for Neovim plugins.

kylechui commented 1 year ago

Notes to self:

kylechui commented 1 year ago

More notes to self: