Autoformat lua_ls & stylua discrepancies.

[zwergius]

Describe the bug

I recently forked the project and have been adding my personal touches and enabled require 'kickstart.plugins.autoformat'

This made it impossible for me to commit my changes as the "Check Lua Formatting" workflow would fail.

This is due to all Lua files being formatted with lua_ls rather than stylua.

I thought this was a bit inconvenient.

To Reproduce

1. Fork & clone the repo
2. Enable require 'kickstart.plugins.autoformat'
3. Make a change in init.lua and save (Should now be auto formatted with lua_ls)
4. Commit and push changes, see that workflow fails.

Desktop

• OS: MAC
• Terminal: Alacritty

Neovim Version

:version
NVIM v0.9.4
Build type: Release
LuaJIT 2.1.1703358377

[feoh]

@dam9000 Do you have thoughts on this? The stylua workflow was something you'd worked on I think?

[feoh]

One thought, could we change the autoformat to use stylua instead?

I see that this exists.

[dam9000]

@feoh right, I added the github stylua workflow, but the actual .stylua.toml was not my work.
If someone prefers to use a different styling then I would suggest that he also removes the stylua workflow in his own fork, that is removing the .github/workflows/stylua.yml file.
Although on the other hand if the autoformat is the actual example in kickstart, then we might indeed make an effort to synchronize the formatting options or logic to be the same.

[feoh]

Hi @zwergius - After looking into this I don't personally think there's an inconsistency here.

The autoformat feature uses your language server - in this case lua_ls - to autoformat your code.

The community has asked that we use Stylua to format the code in this project, so that's what we're doing.

Your choices are:

• Turn off autoformat and format your code with stylua instead. You can use stylua.nvim to do this automatically I think.
• Modify the autoformat feature to somehow optionally use stylua instead? The code is here

I'd think #1 would be the easiest. I'm closing this bug for now, but feel free to re-open and submit a PR if you can find a way to make things more consistent for you.

[zwergius]

Hi @feoh

I have to say that I disagree - this issue is reproducible on a bare bone fork of this repo, as lua_ls is what is included and when enabling the autoformat feature also provided by the project this issue occurs.
It's not a huge issue in that I agree, but I do think that it's unfortunate to include a autoformatting example, that if you enable it you won't initially be able to merge your code in Github. It wasn't straight forward figuring out why I all of a sudden could not merge my commits.

I have a temporary solution in place already running npx @johnnymorganz/stylua-bin **/**.lua before I commit.

The autoformat already opts out of default autoformatting when tsserver is active

        -- You can remove this line if you know what you're doing :)
        if client.name == 'tsserver' then
          return
        end

So something similar should be fairly simple, just hard to figure out when you're entirely new with this project.

Anyways this issue will be around for anyone running into this issue in the future.

[feoh]

OK. I'm not trying to be a dictator or anything here :-)

what would you like to see happen? Can you please provide details to the changes you would like to see made? Thanks!

[zwergius]

Hi again,

So as far as I see it there are 3 options:

1. Change Github action to use lua_ls for formatting. (Not sure this is possible), but best option.
2. Change autoformat.lua to opt out for lua_ls just like it's done for tsserver, this would mean that autoformat.lua would also need to install stylua
3. Add a .editorconfig which is a config file for lua_ls - configuring it to give the same output as stylua

[feoh]

OK, I'm re-opening this because it seems like we have some valid concerns to resolve here.

@zwergius you want the inconsistency between autoformat and our stylua Github check-in check resolved. That seems reasonable. Honestly, none of your proposed options seem workable to me. Silently ignoring the user when autoformat is enabled seems not great, making the Github action use lua_ls formatting feels very impractical, and I don't even know what the .editorconfig route would entail. It seems to me that the only reasonable way to get what you want is to eliminate the Stylua check.

@dam9000 - you proposed (I think?) and created the Stylua check. I've never had strong feelings on this although in general I am in favor of having a single style for all submitted code. I'd be just as happy with a note saying "Please run stylua on your code before submitting." though :)

So, I have no strong feelings on any of this, it seems to me like you two should figure out a way to resolution on this issue, but I would also suggest that you should probably be willing to back whatever solution you decide upon with a willingness to do any necessary work yourselves or get someone from the community to do it, because to me all of this is ancilliary to this project's primary mission.

Acceptable outcomes as I see it:

1. Close this issue. @zwergius will agree that this inconsistency is one we can live with given the current state of the art of Lua, Github, etc.
2. We remove the Stylua Github check. @dam9000 would need to be OK with this as the primary author (I think?) of this change.
3. Some other technical solution that makes everyone happy that I haven't thought of yet :)

Thanks for your input and contributions everyone, let's please figure this out so we can get back to providing a quality starting point for folks to embark on their Neovim customization journey :)

[zwergius]

Hi again,

Yea so this made me think - maybe we could do as here https://github.com/orgs/community/discussions/26704#discussioncomment-3252979 and make sure this action is only run when in nvim-lua/kickstart.nvim that should make all parts happy and the main repo can still benefit from having this check.

[dam9000]

Opened a PR with the last @zwergius suggestion:

#609

[feoh]

@dam9000 Thanks so much for your fix! And @zwergius thanks for being patient while we sorted this out. I'm resolving this because I just merged that change.