Misc. changes + Go major mode (that follows no real Spacemacs layers but "feels right" to me)

[macintacos]

I added a Go major mode that I initially wanted to make for VSpaceCode proper, however the existing Go spacemacs layer is really.... messy in its organization (IMO). It doesn't help that I started Go development only after I stopped using Spacemacs, so looking at that layer was pretty alien to me.

Since VSpaceCode is trying to keep to what Spacemacs users are familiar with, I opted to add them to my config first and then see what the maintainers think :)

[stevenguh]

It seems like i is often used in other layers for import, and as well for inserting, although, I understand it is a bit weird that it has a remove command

[macintacos]

Ah, I'm cool with changing this to "i" - I just kept seeing the word "add" and just felt like it needed to be an "a", but I can switch this over to "i" if it's a convention most folks are familiar with

[stevenguh]

Make sense. I guess I will treat this as review for the go binding? I think the go binding makes sense for the most part.

As far as the major mode bindings, the expectation is try to match the convention like r for refactoring, and i for inserting. However, it is expected that a lot of the command offered by the language extension is not going to match with the layers in spacemacs :)

[marcoieni]

I will review this soon

[marcoieni]

This is so huuuuge, as always thank you so much!
However I don't have time/energy to review every single key binding ^^'
I am mostly just checking letters of first hierarchy because I would like to keep them consistent, at least.

[marcoieni]

I would remap this to spc b r. In fact, if you check the lsp docs you will see that this is very similar to the "lsp-workspace-restart" action.
I wasn't able to get this binding in go (I don't use go, so I just installed the spacemacs layer and nothing else, therefore my environment might be broken), but for example in rust I see this option.
spc b stands for backend.

[stevenguh]

I thought about this too, it doesn't seem like there is a consistent naming across different layers. For example, Python layer uses v to represent environment (I assume from venv), C# layer has s to represent OmniSharp (its language server). If b more common, maybe we should just use b?

[marcoieni]

b is different from environment (I think). I was talking just about this key binding.
I was ok with the other SPC e section.
But I am not a go developer as I said.
b is related to the language server protocol, I think we should use the lsp key bindings everywhere because they are common on almost every language (I think).

[stevenguh]

Ah got you, that makes sense

[macintacos]

Fair - I was using e since it corresponded with "environment", and I made the stretch of SPC f e R (restarting your editor config) to be synonymous with "restarting something" (in this case, restarting the language server). If this is a typical keybinding for LSP, I'll move over to using that, but does that mean everything else should move over there as well? Would feel like a weird split I think.

[macintacos]

Unless I hear a strong objection, I'm just going to change the e menu to b, since that's more in-line with what the LSP docs expect. Will push that shortly.

[marcoieni]

For me that's it for the review. Again, I haven't checked everything.
I hope go developers who will use this everyday will discuss with you these key bindings in the future after it will be merged :)
Do you plan to do a PR with these key bindings in the main repo?
Anyway, for the next time it might be better to just do a PR, so we all do the work once :)

[macintacos]

I was planning on doing a PR in the main repo if y'all didn't have any objections to the way I was structuring things - I didn't want to put all the effort in just for you to think that I should go in a completely different direction 🙂

In the future, I'll just open the PR - no worries!

[macintacos]

RFAL

[stevenguh]

Thank you for the quick turnaround. How about we merge this, and review the one when you open a PR in the main repo?

btw, in the main repo, the object key order should probably follow the rest of the bindings like in the order "key", "name", "type", "command" to be consistent.

RFAL

What does this mean?

[marcoieni]

How about we merge this, and review the one when you open a PR in the main repo?

It doesn't make sense because in his config he will delete this once the changes are in the main repo I think.

So we should just close this pr I think.

Macintacos, when you do the PR, please follow alphabetical order, thanks.

Rfal means ready for another look I think. It took me a while to figure it out 😂

[marcoieni]

Or you could delete the go part from this pr and keep the other key bindings, and we merge it.

Anyway I reviewed your changes. They look good to me

[stevenguh]

Either way works for me :) just a suggestion.

Rfal means ready for another look I think. It took me a while to figure it out 😂

That makes sense 😅

[macintacos]

Oh darn sorry, I use "RFAL" all the time at work (damn you jargon) - should be more clear about that in the future 😓

Sure I'll move forward with removing the "Go" bindings and get that update out to get this merged, and will open a PR in VSpaceCode as soon as I can! Thanks!

[macintacos]

Oh and last bit - the ordering I have in my config is because I sort all JSON alphabetically in my own projects by key name (checking out my settings.json if you're curious). I'll make sure I keep it consistent with other mappings in VSpaceCode proper when I open that PR 🙂

[macintacos]

Ready For Another Look 🙂

[stevenguh]

I just going to merge it since you are the owner of this config :) Feel free to update this whenever you need.