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

docs: fix lua codes, consistent nerdfonts symbols, formatting #922

Closed
wants to merge 6 commits into from

Conversation

gepbird
Copy link
Contributor

@gepbird gepbird commented Jun 9, 2024

The last formatting commit 7eb3326 has a much bigger diff than the other commits, please review commit-by-commit rather than all commits at the same time to make it easier for yourself :)

Let me know if you prefer another style for lua code in docs, I mostly followed the project's stylua.toml with a few exceptions:

  • 4 space indent (this was more common than 2 spaces)
  • don't merge if statements to one line

It tried to compare bool with bool which is invalid lua.
Previously a new highlight was created and cached for each index.
For example when the result table only has a warning, an orange color
highlight will be created and cached for index 1. Then the user fixes
the warning but generates an error with index 1, the orange color will be
reused from the cache rather than creating a highlight for the
associated red color.
@akinsho
Copy link
Owner

akinsho commented Jul 18, 2024

Hi @gepbird,

These appear to be a lot of stylistic changes mixed in with some more important fixes. Can we just address the crucial fixes first in a minimal PR just aimed at those then discuss any style changes as a followup. In general a lot of the attempts to add booleans and things like that are actually contrary to my intentions. The reason I added them like that was actually to specifically discourage copy-pasting which ends up being a huge cause of issues for me since if the code is clearly not copy-pastable people tend to add things in themselves rather than copy a huge blob then complain it doesn't work as they expect

@gepbird
Copy link
Contributor Author

gepbird commented Jul 18, 2024

Sure, I'll break these changes up to multiple PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants