refactor list creation functionality #3
Conversation
mcauley-penney
commented
Jun 25, 2022
By formatting all of the icon lists, it
becomes possible to remove a good deal of
repetition in the list-building code and
automate a great deal. For example, by
making all icon tables formatted the same
way, instead of having many functions to
build lists, we can have one function that
does the same work. We can also automate
the user command creation because of how
similar all of the user command creation
calls are
|
Hey, how's it going! So, I formatted the icon sets and that allowed for a dramatic refactoring, eliminating a lot of repetition. This code is more extensible, I think, because new lists can be created without the addition of new functions. It is arguably less readable, though, and I think that the variable names could be improved a lot. I think it is WIP but I submitted it so that you could take a look and see if you 1. liked the changes and 2. if you did like them, what you would like to change. |
• unroll nested loop in command Initialization
• eliminates `callback_tbl`
• makes "Insert" command creation more explicit
• remove dynamic cmd name creation
• move "Pick" substring into `list_types` keys
• makes the command creation process easier to
follow because the command names are more
complete and explicit in the source
• alphabetize tables
• reformat with default stylua settings
• delete a few empty lines
|
Oh wow, I'm sorry I've taken this long to reply. I'll take a look at this :) |
|
This is my first time receiving a complex PR. I'll look into how to handle PRs & test things properly :) |
No worries, I totally understand! I usually try and start with smaller PRs when I'm working with someone I've never worked with before but, after I started on this, I kind of just kept going because it was fun and its a neat project. I tested it out as best I could but I would be happy to correct any problems that you find. Importantly, you obviously don't need to accept all of the changes I've made. I wrote and submitted this knowing that it was a lot of changes and I expect that at least some of them won't appeal to you, so I totally understand not wanting to accept all of them into the main branch. |
|
Yeah I'm currently busy adding features to color-picker.nvim. I'll do this PR properly when I'm finished with it (soon™). |
|
I really appreciate your PR 👍. Unfortunately I will need to consult my dear friend who has much more experience than me, which will make the process take a bit longer. |
|
There's no reason to apologize! It sounds like a good plan to me 👍🏻 |
|
Hi man, I managed to pull down your code locally with github cli. And your changes are amazing! I'm merging this soon 👍 . |
|
I've learned so much from your PR. Thank you so much ❤️ ! |
That's awesome, I'm really glad to hear that! If there are any bugs that crop up or issues that are opened that concern this PR, I'd be more than happy to be assigned to them. Again, I'm really glad that this worked out and that you liked it! |
|
Cheers man :) Take care. |