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

Refactoring codebase #61

Closed
kevinhwang91 opened this issue Feb 1, 2022 · 13 comments · Fixed by #64
Closed

Refactoring codebase #61

kevinhwang91 opened this issue Feb 1, 2022 · 13 comments · Fixed by #64
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@kevinhwang91
Copy link
Contributor

It sounds harsh, but I like the idea of this plugin. If you don't mind, I will make a PR with help.

@danymat
Copy link
Owner

danymat commented Feb 1, 2022

No, the code is a little bit messy lol (I did not have a good understanding of Lua at the time)
I think the code that is messy is really inside the granulator and generator, so maybe we could start from there.
I'm currently improving the templates part, and providing some docs and API for it.

Feel free to open a PR, and let me know what do you plan to refactor ;)
Thanks for your contribution !

@danymat danymat changed the title I think the code is messy and need refactor Refactoring codebase Feb 1, 2022
@danymat danymat added enhancement New feature or request help wanted Extra attention is needed labels Feb 1, 2022
@kevinhwang91
Copy link
Contributor Author

I'm looking cursor.lua, just using a singleton table for the positions is enough. Using extmarks is overkilled, less stable, and has more side-effects for Neovim.

@danymat
Copy link
Owner

danymat commented Feb 1, 2022

Sure, thanks for your contribution ! 💯

@kevinhwang91
Copy link
Contributor Author

kevinhwang91 commented Feb 1, 2022

Are you using extmarks because you want to support inserting a new line for the comments? If yes, I think must use extmarks or marks, but extmarks are better than the limited marks.

@danymat
Copy link
Owner

danymat commented Feb 1, 2022

What do you mean exactly ?

@kevinhwang91
Copy link
Contributor Author

Once the template is generated, the positions are computed that are immutable, but the position of extmarks are mutable.

:h extmarks

Extended marks (extmarks) represent buffer annotations that track text changes
in the buffer. They can represent cursors, folds, misspelled words, anything
that needs to track a logical location in the buffer over time.

@danymat
Copy link
Owner

danymat commented Feb 1, 2022

Oh yeah, I use extmarks because we can jump between annotation types even after we create some annotation and modify the lines

@kevinhwang91
Copy link
Contributor Author

Thanks, I will deep into the codebase and refactor it later, maybe tomorrow or the day after tomorrow.

@danymat
Copy link
Owner

danymat commented Feb 1, 2022

No problem, take your time, feel free to ping me if you need anything

@kevinhwang91
Copy link
Contributor Author

Why

elseif type(template) == "function" then
may be a function type? I can't find and grep any information about this.

@danymat
Copy link
Owner

danymat commented Feb 3, 2022

Why

elseif type(template) == "function" then

may be a function type? I can't find and grep any information about this.

Yes, during initial development it was assumed we could pass templates as functions, but I guess we'll not use them anymore. Even the granulator and generator, I'm not really found of creating your own.
Maybe we could simplify the process by making granulator and generator a default one, and only make templates and locators modifiable.
Because right now, the generator is used everywhere and with some special options..

@kevinhwang91
Copy link
Contributor Author

kevinhwang91 commented Feb 3, 2022

Why

elseif type(template) == "function" then

may be a function type? I can't find and grep any information about this.

Yes, during initial development it was assumed we could pass templates as functions, but I guess we'll not use them anymore. Even the granulator and generator, I'm not really found of creating your own. Maybe we could simplify the process by making granulator and generator a default one, and only make templates and locators modifiable. Because right now, the generator is used everywhere and with some special options..

Thanks, refactor is going to be done, WIP in my local repo.

Maybe we could simplify the process by making granulator and generator a default one, and only make templates and locators modifiable.

I'm doing this idea, but I have no idea how to write the doc and tell the users what's the change is breaking.

@danymat
Copy link
Owner

danymat commented Feb 3, 2022

I'm doing this idea, but I have no idea how to write the doc and tell the users what's the change is breaking.

No worries, I will update the docs in your pr before merging

@danymat danymat linked a pull request Feb 6, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants