Skip to content

Conversation

@zachallaun
Copy link
Contributor

This PR organizes the extension's config settings and renames the Next LS setting prefix to improve setting legibility.

Summary of changes:

  • Split settings into "Next LS" and "Credo LS" section, in that order.
  • Add an "order" attribute to the *.enable properties so that they come first.
  • Rename "elixir-tools.nextls.*" to "elixir-tools.nextLS.*". This causes properties to render as "Next LS: *" instead of "Nextls: *".

Before:

settings-before

After:

settings-after

@zachallaun
Copy link
Contributor Author

Note that I can't currently run NextLS and merging this should wait :)

@mhanberg
Copy link
Contributor

mhanberg commented Aug 9, 2023

Nice!!

I think I have a hack to get it to work if you want to try, someone is currently seeing if they can fix it to run the new prebuilt binaries.

If you download the binary from the releases page for your OS and architecture and replace the bin/nextls file inside the extension with it, it will most likely work

For example on my macbook, the path is /Users/mitchell/.vscode/extensions/elixir-tools.elixir-tools-0.5.0/bin/nextls

so if you download the new binary, rename to nextls, and replace the existing file at that path, hopefully you see success

@mhanberg mhanberg changed the title Reorganize configuration settings refactor!: reorganize configuration settings Aug 9, 2023
@mhanberg
Copy link
Contributor

@zachallaun friendly reminder to rebase and test this and we get it merged and released 💅

@zachallaun
Copy link
Contributor Author

@mhanberg Apologies for the delay! This should now be ready for review.

@zachallaun
Copy link
Contributor Author

zachallaun commented Aug 25, 2023

Fixed! 22fba70..22b34f3

package.json Outdated
"properties": {
"elixir-tools.nextLS.enable": {
"type": "boolean",
"default": false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also on main i changed this to true

package.json Outdated
"properties": {
"elixir-tools.credo.enable": {
"type": "boolean",
"default": true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on main i changed this to false

@mhanberg mhanberg merged commit fc3154d into elixir-tools:main Sep 18, 2023
@mhanberg mhanberg deleted the reorg-settings branch September 18, 2023 18:57
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