AstroNvim / astrocommunity

A community repository of common plugin specifications
GNU General Public License v3.0
1.04k stars 214 forks source link

feat: add several new plugins and modularize the clojure pack #982

Closed practicalli-johnny closed 1 month ago

practicalli-johnny commented 1 month ago

📑 Description

Provide both approaches to structural editing Clojure code (paredit & parinfer) and remove the more opinionated configurations for Conjure REPL client.

ℹ Additional Information

Conjure plugin will disable diagnostics in the REPL log (HUD), removing the need to define the autocmd to disable them.

If you prefer I submit this as 4 separate PR's then let me know...

github-actions[bot] commented 1 month ago

Review Checklist

Does this PR follow the [Contribution Guidelines](development guidelines)? Following is a partial checklist:

Proper conventional commit scoping:

Uzaaft commented 1 month ago

I believe you should remove the guides on how to configure the different plugins. The astrocommunity docs shouldn't serve as a guide on how to configure or make them work. That's prone to be outdated across updates.

practicalli-johnny commented 1 month ago

I am a bit surprised we don't want to help people with some more docs, as it took me a while to figure this out. Can we link to an external guide?

If I can't specify how to disable either paredit or parinfer, then I should create 3 Clojure packs.

  1. Clojure + paredit + parinfer
  2. Clojure + paredit
  3. Clojure + parinfer

Then people can choose which approach they use.

Or I could remove paredit & parinfer and make the Clojure pack incomplete by itself. I assume suggesting other Astro community plugins is acceptable in the readme.

practicalli-johnny commented 1 month ago

FYI. It seems some of the GitHub actions have put of date libraries and are not running.

Uzaaft commented 1 month ago

I am a bit surprised we don't want to help people with some more docs, as it took me a while to figure this out. Can we link to an external guide?

If I can't specify how to disable either paredit or parinfer, then I should create 3 Clojure packs.

  1. Clojure + paredit + parinfer
  2. Clojure + paredit
  3. Clojure + parinfer

Then people can choose which approach they use.

Or I could remove paredit & parinfer and make the Clojure pack incomplete by itself. I assume suggesting other Astro community plugins is acceptable in the readme.

Sure link to external docs is fine.

We just don't want the burden of keeping things updated, or correct as plugins tends to have changes, and over time docs gets updated. Adding the docs in this repository could also be interpreted as "Hey, let me ask them my question", which we also don't want(it has happened before). Most of your additions in the docs are just substitute to the plugin docs, which the end-user can look up(and should).

We can wait on response from @mehalter on this as well. But I don't like the precedence it sets if we also serve as a substitute for the documentation of the plugin.

Uzaaft commented 1 month ago

FYI. It seems some of the GitHub actions have put of date libraries and are not running.

It's working as expected.

mehalter commented 1 month ago

@practicalli-johnny I am going to pull this branch and clean it up directly. So no need to fix these things I commented on, I will fix them and test locally.

mehalter commented 1 month ago

Ok, I just pushed a refactor. A lot of the pieces of the Clojure pack were not specific to clojure at all and are generally useful for many languages. Seems to make sense to refactor those out as to not end up duplicating the code around AstroCommunity and then just importing them in the pack. Let me know what you think @practicalli-johnny !

I also removed the changing of default comment strings in a couple plugins to ;;. This seems more like a style choice and the default commenting in neovim comments things as ; anyway so it seems like a decent default and then it can be up to the user to reconfigure all the plugins to comment with ;; accordingly.

mehalter commented 1 month ago

Also this touches several scopes so the CI will fail. I will handle merging this manually once we get approval from all parties. But I tested locally and it seems to all be working

mehalter commented 1 month ago

Oh and for what it's worth, I think having all the documentation in there does nothing but help people. I don't think it should be considered a requirement, but if someone wants to add a bunch of documentation then we should only be grateful ❤️

practicalli-johnny commented 1 month ago

Okay, thanks for the assistance. I am happy to help if I can, although still learning a lot about Neovim and Lua.

I can add docs and examples to https://practical.li/neovim rather than the readme if it helps lower the maintenance of the pack

mehalter commented 1 month ago

I think it's ok where it is now! It's nicely discoverable and the AstroCommunity maintainers don't actually have any maintenance burden. It's not up to them to keep things updated and working, their only real responsibility is manage the issues and approve pull requests :)

mehalter commented 1 month ago

@practicalli-johnny let me know when you get a chance to look over things and give it a test and I can merge this in :)

practicalli-johnny commented 1 month ago

I also removed the changing of default comment strings in a couple plugins to ;;. This seems more like a style choice and the default commenting in neovim comments things as ; anyway so it seems like a decent default and then it can be up to the user to reconfigure all the plugins to comment with ;; accordingly.

According to the Clojure Style guide then ;; is the full line comment and ; is the end of line comment.

Most editors adopt this as default, but unfortunately it's not universal and causes format issues when loaded by those editors that do. However, adding the ts-comment.nvim could be mentioned in the readme to help people follow the Clojure style guide.

I'll review the updates after work, thanks for all the help.

UPDATE: A PR merged today int ts-comment.nvim makes this point satisfactorily resolved.

luxus commented 1 month ago

@practicalli-johnny so you are writing your config with nlfnl? you should join our https://discord.astronvim.com

practicalli-johnny commented 1 month ago

Thank you for all the feedback and changes.

Having these tools as separate configs is excellent approach, with the Clojure pack bringing them all together.

I've tested the updated Clojure pack on some Clojure projects and all looks good.

I've simplified the documentation and moved the lengthy examples to Practical.li Neovim - Astrocommunity to keep maintenenace of Astrocommunity low. I've included a link to these examples in the Clojure pack readme.

I've updated ts-comment.nvim via a PR to support the Clojure language, so that configuration is no longer needed anyway.

I'll update the Practicalli AstroNvim User Config with my own preferences once this PR has been approved.

Thanks again for all the help and feedback. I am happy to have this code merged if everyone else is okay with it.

Uzaaft commented 1 month ago

@practicalli-johnny Amazing work!