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

Array item properties are created with the wrong indent #192

Closed
itowlson opened this issue Jul 9, 2019 · 27 comments · Fixed by redhat-developer/yaml-language-server#172
Closed
Labels

Comments

@itowlson
Copy link
Contributor

itowlson commented Jul 9, 2019

I have a JSON schema which contains an array property. The array item type is an object with properties. The desired layout is like this:

install:
  - helm:
      name: foo
  - helm:
      name: bar

When I add the array using tab-completion, RH YAML correctly inserts a starting array
element with the correct indentation:

in<HIT TAB TO AUTOCOMPLETE>

=>

install:
  - helm:
      name: <CURSOR POSITIONED HERE>  # This is correct

But when I add a second array element by tab-completing the name, it does not indent the sub-properties:

install:
  - helm:
      name: foo
  - he<HIT TAB TO AUTOCOMPLETE>

=>

install:
  - helm:
      name: foo
  - helm:
    name: <CURSOR HERE>   # Note name is *NOT* indented from its parent

Furthermore, our real scenario is that the array can contain multiple different kinds of item (which is why we have the helm parent name rather than just an array of tuples). So our JSON schema specifies "items": { "anyOf": [ { "$ref": "...helm..." }, { "$ref": "...exec..." }, ... ] }. In this case, even with only a single option in the "anyOf", no element is auto inserted (correctly, as RH YAML could not know which of the anyOf options to take), and the user must explicitly insert the first array element, which is then indented incorrectly

# With the anyOf variant of the JSON Schema

in<HIT TAB FOR AUTOCOMPLETION>

=> 

install:
  - <CURSOR HERE>  # This is okay

=>

install:
  - he<HIT TAB FOR AUTOCOMPLETION>

=>

install:
  - helm:
    name: <CURSOR HERE>  # Incorrect indentation

Here is the simplified JSON Schema which I used to reproduce the issue:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "definitions": {
    "installStep": {
      "properties": {
        "helm": {
          "additionalProperties": false,
          "properties": {
            "name": {
              "type": "string"
            }
          },
          "required": [
            "name"
          ],
          "type": "object"
        }
      },
      "required": [
        "helm"
      ],
      "type": "object"
    }
  },
  "properties": {
    "install": {
      "items": {
        "anyOf": [
          {
            "$ref": "#/definitions/installStep"
          }
        ]
      },
      "type": "array"
    }
  },
  "type": "object"
}

Note that this is the ‘anyOf’ schema to show the second part of the issue (which is our real environment). To reproduce the first part of the issue, remove the anyOf from around the $ref in the items schema (or I can send you that schema too).

Please let me know if you need any more information.

@JPinkney JPinkney added the bug label Jul 10, 2019
@JohannLai
Copy link

I had the same problem.

@squillace
Copy link

@JPinkney should we submit a PR?

@JPinkney
Copy link
Contributor

@squillace Yes preferably

@andxu
Copy link
Contributor

andxu commented Jul 23, 2019

hi @itowlson , I have a different result using the latest vscode and RH-yaml on marketplace, did I misunderstand something?

test

@itowlson
Copy link
Contributor Author

@andxu You're definitely getting the output we want. Could you post your schema or a screenshot of your schema please? It's disappearing too quickly for me to read it off the GIF - sorry.

@andxu
Copy link
Contributor

andxu commented Jul 24, 2019

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "definitions": {
    "installStep": {
      "properties": {
        "helm": {
          "additionalProperties": false,
          "properties": {
            "name": {
              "type": "string"
            }
          },
          "required": [
            "name"
          ],
          "type": "object"
        }
      },
      "required": [
        "helm"
      ],
      "type": "object"
    }
  },
  "properties": {
    "install": {
      "items": {
        "anyOf": [
          {
            "$ref": "#/definitions/installStep"
          }
        ]
      },
      "type": "array"
    }
  },
  "type": "object"
}

@squillace
Copy link

Thanks so much for following up on this @andxu. @itowlson?

@itowlson
Copy link
Contributor Author

@andxu That looks right for the repro. Let me check I'm up to date with everything and try again. Thanks!

@itowlson
Copy link
Contributor Author

@andxu I'm on VS Code 1.36.1 and YAML 0.4.1. Those both look up to date to me. And I pasted your schema into my schema content callback. But I'm still seeing the problem. I wonder what's different between our two environments?

@JohannLai I saw you thumbs-upped Andy's schema - is that working for you, or still giving the same problem?

@itowlson
Copy link
Contributor Author

@andxu I noticed your entries are indented by 4 whereas mine are indented by 2. I am clutching at straws here, but are you using tabs or spaces? Where is your indent configured?

@itowlson
Copy link
Contributor Author

@andxu If I switch to tabs I get the desired behaviour as shown in your GIF (except for a wiggly saying 'tabs can lead to unpredictable results'). Could you try switching to spaces and see if that reproduces the error for you please?

@itowlson
Copy link
Contributor Author

@andxu The plot thickens. It's not tabs vs. spaces. It's indent size 2 vs. indent size 4. I have indent size 2, and get the misalignment. If I change it to 4, everything works. (And if I use tabs, but with a size of 2, it goes wrong again.)

@JohannLai
Copy link

I'm not sure it works. I think it's worth a try. I'll try it later.

@itowlson
Copy link
Contributor Author

@andxu @squillace got confused about which indent I was referring to because apparently the YAML extension has tab conversion stuff of its own, so for avoidance of doubt it is this one - works when set to 4, shows the bug when set to 2. No YAML-extension-specific settings in play.

image

@squillace
Copy link

@andxu I repro @itowlson's explanation. I have a default of 2 spaces on my VS Code, and I get the following when using with Spaces: 2. This is incorrect autocomplete.

image

if I then switch to Spaces: 4 I get the following, which is what you get.

image

@andxu
Copy link
Contributor

andxu commented Jul 25, 2019

Looks funny, I will test it on different indents to figure out the root cause.

@squillace
Copy link

:-) Thanks, @andxu, thanks so much for tolerated us. :-) Let us know what you find!

@andxu
Copy link
Contributor

andxu commented Jul 25, 2019

When this plugin provides completion text for the sample yaml below, the snippet vscode-yaml provides is "helm:\n\tname: $1" with insertTextFormat(in the case of error indents: 2), which will result in the final code change of "text": "install:\n■■-■helm:\n■■■■name:■"(I replaced spaces with '■'), if I change the indents to 4, the same snippet will result in "install:\n■■-■helm:\n■■■■■■name:■", seems the code here makes the snippet code, I will continue to research more on vscode's behavior on applying code snippet.

install:
  - he<HIT TAB TO AUTOCOMPLETE>
``

```json
[Trace - 4:38:51 PM] Received response 'textDocument/completion - (43)' in 2ms.
Result: {
    "items": [
        {
            "kind": 10,
            "label": "helm",
            "insertText": "helm:\n\tname: $1",
            "insertTextFormat": 2,
            "documentation": "",
            "textEdit": {
                "range": {
                    "start": {
                        "line": 1,
                        "character": 4
                    },
                    "end": {
                        "line": 1,
                        "character": 6
                    }
                },
                "newText": "helm:\n\tname: $1"
            }
        }
    ],
    "isIncomplete": false
}


[Trace - 4:38:51 PM] Sending request 'completionItem/resolve - (44)'.
Params: {
    "label": "helm",
    "insertTextFormat": 2,
    "textEdit": {
        "newText": "helm:\n\tname: $1",
        "range": {
            "start": {
                "line": 1,
                "character": 4
            },
            "end": {
                "line": 1,
                "character": 6
            }
        }
    },
    "kind": 10
}


[Trace - 4:38:51 PM] Received response 'completionItem/resolve - (44)' in 0ms.
Result: {
    "label": "helm",
    "insertTextFormat": 2,
    "textEdit": {
        "newText": "helm:\n\tname: $1",
        "range": {
            "start": {
                "line": 1,
                "character": 4
            },
            "end": {
                "line": 1,
                "character": 6
            }
        }
    },
    "kind": 10
}


[Trace - 4:38:51 PM] Sending notification 'textDocument/didChange'.
Params: {
    "textDocument": {
        "uri": "file:///c%3A/azure-webapp/helloworld/test.yaml",
        "version": 34
    },
    "contentChanges": [
        {
            "text": "install:\n  - helm:\n    name: "
        }
    ]
}

@itowlson
Copy link
Contributor Author

@andxu Thanks for the update. I wonder if it’s also worth looking at what is different about the case where the array item type is specific instead of an anyOf - per the initial bug report, this caused the FIRST array element to indent correctly even with an indent if 2... but still got it wrong on subsequent elements...

@andxu
Copy link
Contributor

andxu commented Jul 25, 2019

The indent settings in vscode will cause the spaces(if you don't use 'space' other than 'tab') added at the
baseline of the first '-' in -helm, so it gives the following result:

# indent 1
install:
- helm:
^name: 
# indent 2
install:
- helm:
^^name: 
# indent 3
install:
- helm:
^^^name: 
# indent 4
install:
- helm:
^^^^name: 

as the test case, if the start of helm is far away from slash, indents 4 will also fail:

install:
-              hel<HIT TAB TO AUTOCOMPLETE>

will result in

# indent 4
install:
-              helm:
    name: 

@andxu
Copy link
Contributor

andxu commented Jul 25, 2019

So I think it is a bug in yaml language server generating the snippet at the wrong indent on array objects.

@andxu
Copy link
Contributor

andxu commented Jul 29, 2019

I will post a PR before 08/05.

@itowlson
Copy link
Contributor Author

itowlson commented Aug 7, 2019

Thanks for the fix @andxu (and for the review @JPinkney)!

@andxu
Copy link
Contributor

andxu commented Aug 9, 2019

once the yaml LS is published at npm, I will update the vscode-yaml here.

@andxu
Copy link
Contributor

andxu commented Sep 3, 2019

thank @JPinkney for publish a new version.

@konsumer
Copy link

I have same prob with "Reindent Lines"
Peek 2020-08-21 15-33

@JPinkney
Copy link
Contributor

Looks that that can be reproduced without this extension installed

services:
  test:
    build:
    ports:
      - 8080
    networks:
      - test

after re-indent lines

services:
  test:
    build:
      ports:
        - 8080
        networks:
          - test
          

bleach31 pushed a commit to bleach31/vscode-yaml that referenced this issue Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants