hrsh7th / nvim-cmp

A completion plugin for neovim coded in Lua.
MIT License
8.1k stars 402 forks source link

Please, comment code, this is a major issue #1429

Open hinell opened 1 year ago

hinell commented 1 year ago

FAQ

Announcement

Minimal reproducible full config

n/a

Description

Please, comment your code, this is a major issue. When I got through it I'm puzzled by some of its parts. Still can't figure where config is acessed. This takes my time and prevents me from contributing.

Thanks!

Steps to reproduce

n/a

Expected behavior

n/a

Actual behavior

n/a

Additional context

No response

Shougo commented 1 year ago

I don't understand your issue.

Diegovsky commented 1 year ago

The issue raised is @hinell wants nvim-cmp to have more documentation internally for them to contribute.

hinell commented 1 year ago

Would be nice to have a description of the plugin lifecycle! Thanks!

ijsnow commented 1 year ago

Putting in the time to understand the code and documenting the code with what you learned would be great first contribution to an open source project. It's cool that you want to contribute to the ecosystem but this is neither a bug nor a helpful issue. I'm sure the maintainers would answer specific questions about parts of code but you have to remember that they are working on this in their free time and documenting internals that not very many people are going to see (and no one HAS to), is not exactly a priority for a free and open source editor plugin.

hinell commented 1 year ago

I think this should be more like a project long-term milestone.

laurentS commented 1 year ago

Maybe adding some checks in CI to make sure all new PRs going forward have comments in the code, like a minimal docstring for each function, could help improve the situation?

hrsh7th commented 1 year ago

I don't think adding comments to the current codebase would make anything better. Adding something like a diagram might be a good idea.

BTW, there are plans to separate the core implementation. There is an implementation in the repository shown below. (still on the way) This implementation is conceptually identical to the current nvim-cmp, so you might be able to figure it out by reading here.

If this repository doesn't tell you either, I have no other ideas. https://github.com/hrsh7th/cmp-core-example/tree/main/lua

google translated

hinell commented 1 year ago

@hrsh7th That's fine. If you are interested, you can use something like graphviz to quickly sketch-up a diagram architecture of your project.

If this repository doesn't tell you either, I have no other ideas.

Well, you should add a brief readme on what's going on out there.

milanglacier commented 1 year ago

Why not try to read the code and improve the documentation? I am sure the maintainer will happy to merge your PR about the improvement of the doc.

People working on the projects on their free time, especially the main maintainer, if this is not a bug, there’s no reason that you criticize the main maintainer with such stance.

hinell commented 1 year ago

@milanglacier Well, there is no ciriticism actually. This is rather a feature request. Underdocumented code, without any hints is hard to document because I can't know the architecture in first place. Neither can I know the intent of the original dev.

The issue can be safely closed though. I might consider to open a PR in the future.

litoj commented 1 year ago

I just stumbled upon max_item_count on reddit, but it is not documented anywhere, despite it being a very useful feature for large sources (latex_symbols...).