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

Map key order #839

Merged
merged 6 commits into from
Jan 30, 2023
Merged

Conversation

gorkem
Copy link
Collaborator

@gorkem gorkem commented Jan 24, 2023

What does this PR do?

Introduces a new setting yaml.keyOrder that enforces the alphabetical key order for mappings when set to true.
In addition to validation, it also provides a code action to fix the order for the map

What issues does this PR fix or reference?

redhat-developer/vscode-yaml#95

Is it tested? How?

Adds tests for validation, new settings and code action

Adds a new validation that checks if the keys
of a map are in alphabetical order.  Updates the
tests to cover the new validation.
@coveralls
Copy link

coveralls commented Jan 24, 2023

Coverage Status

Coverage: 83.05% (-0.05%) from 83.103% when pulling 37505b4 on gorkem:map-key-order into 64e3815 on redhat-developer:main.

Adds a setting to control keyOrdering feature.
Tests if the new setting works as expected.
Starts using the new setting to enable the feature.
Adds a new codeaction that fixes the map order
by rewriting the ast with the alphabetical order
of keys.
Updates readme and changelog with the information
for the new feature.
Adds tests and updates the logic to preserve
comments and spaces.
Copy link
Contributor

@msivasubramaniaan msivasubramaniaan left a comment

Choose a reason for hiding this comment

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

LGTM

@msivasubramaniaan msivasubramaniaan merged commit 832202f into redhat-developer:main Jan 30, 2023
@@ -86,6 +87,7 @@ export class SettingsState {
flowMapping: 'allow' | 'forbid';
flowSequence: 'allow' | 'forbid';
};
keyOrdering = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems this should be false.

@snaggen
Copy link

snaggen commented Apr 13, 2023

When I open a yaml file now in neovim (where I have installed yamlls via Mason and default config) I get a lot of "Wrong order of key" in my docker-compose.yaml and .gitlab-ci.yml files. I have tried to disable it by setting keyOrdering = false but with no improvement.
Also, when looking at https://github.com/neovim/nvim-lspconfig/blob/master/lua/lspconfig/server_configurations/yamlls.lua it sems that they are not enabling this in their default config.

Edit:
Setting this att the latest possible stage seems to fix the neovim issue, since I use mazon-lspconfig it didn't work to set this in the lspconfig, but when I set it in mason-lspconfig it worked. Here is the mason-lspconfig I use with Lazy, just for anywone with the same issue.

 {
        "williamboman/mason-lspconfig.nvim",
        config = function(_, opts)
            require("mason-lspconfig").setup_handlers {
                function(server_name)
                    require("lspconfig")[server_name].setup {}
                end,
                ["yamlls"] = function()
                    require("lspconfig").yamlls.setup({
                        settings = {
                            yaml = {
                                keyOrdering = false
                            }
                        }
                    })
                end
            }
        end
    },

Shourai added a commit to Shourai/nvim that referenced this pull request Apr 18, 2023
@gorkem gorkem deleted the map-key-order branch April 23, 2023 19:05
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.

None yet

5 participants