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

implemented the squeceItem identation based on the sequenceItemIndent… #574

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

msivasubramaniaan
Copy link
Contributor

@msivasubramaniaan msivasubramaniaan commented Nov 3, 2021

What does this PR do?

Doing indentation on sequeceItem based on the settings. If sequenceItemIndent is 0 then the sequenceItem not perform any indentation and it acquire it parent value itself. Suppose sequenceItemIndent contains any values, it will do the indentation on sequenceItem.

  • When sequenceItemIndent is 0
    image

  • When sequenceItemIndent is 2
    image

What issues does this PR fix or reference?

redhat-developer/vscode-yaml#172

Is it tested? How?

Yes. Using test cases

@coveralls
Copy link

coveralls commented Nov 3, 2021

Coverage Status

Coverage increased (+0.009%) to 78.199% when pulling e07db94 on msivasubramaniaan:do-indent-sequenceItem-based-on-settings into 13adb0e on redhat-developer:main.

Copy link
Collaborator

@evidolob evidolob left a comment

Choose a reason for hiding this comment

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

@msivasubramaniaan It doesn't work with:

objA:
  - name: nameA1

and :

objC:
  - { foo: bar }
  - { foo: bar2 }

@evidolob
Copy link
Collaborator

evidolob commented Nov 3, 2021

And I'm not very happy with having own code which do formatting after executing formatter tool. I think it would be better if we change formatter tool or contribute to them new features.
@JPinkney @gorkem WDYT?

@JPinkney
Copy link
Contributor

JPinkney commented Nov 3, 2021

I'd prefer not to own any more of the formatter stuff then we have to. With that said, prettier is very opinionated and probably wouldn't allow us to contribute this feature. Maybe we can switch to prettier-x which is a less opinionated version of prettier and can contribute this feature there

@gorkem
Copy link
Collaborator

gorkem commented Dec 15, 2021

I agree, having to split the formatting logic between the extension and the upstream library is going to cause us more long term issues.

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.

5 participants