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

Incorrect indentation with lists #234

Open
bcoca opened this issue Nov 20, 2018 · 26 comments
Open

Incorrect indentation with lists #234

bcoca opened this issue Nov 20, 2018 · 26 comments

Comments

@bcoca
Copy link

bcoca commented Nov 20, 2018

When using indentation this seems to be applied to the value of the list instead of to the list itself, as you can see below indent=4 is applied after the leading - and not to the list itself.

>>> print(yaml.dump(data['vars']['yaml'], indent=4, allow_unicode=True, default_flow_style=False))
list_of_dict_attr:
-   attr1: value1
    attr2: value2
    attr3:
    - item1
    - item2
single_attr: value1
>>> print(yaml.dump(data['vars']['yaml'], indent=2, allow_unicode=True, default_flow_style=False))
list_of_dict_attr:
- attr1: value1
  attr2: value2
  attr3:
  - item1
  - item2
single_attr: value1

original issue ansible/ansible#48865

@ingydotnet
Copy link
Member

ingydotnet commented Nov 20, 2018

@bcoca can you please check if this is in the Python emitter code or if it's libyaml (ie CDumper) (or both)?

@sivel
Copy link

sivel commented Nov 20, 2018

@ingydotnet Testing with Dumper=yaml.Dumper and Dumper=yaml.CDumper seems to produce the same result.

@ingydotnet
Copy link
Member

Thanks @sivel. This will need to be fixed in both. Patches welcome :)

@znd4
Copy link

znd4 commented Apr 2, 2019

It seems like the problem is occurring because self.indention is set to False in Emitter when expect_block_sequence is run, which makes the self.increase_indent code do nothing.

@DanyC97
Copy link

DanyC97 commented May 15, 2019

@zdog234 i've tried myself the below and didn't make any difference

    def expect_block_sequence(self):                  
        indentless = (self.mapping_context and not self.indention)
        self.increase_indent(flow=True, indentless=indentless)                  
        self.state = self.expect_first_block_sequence_item

or

    def expect_block_sequence(self):                  
        indentless = (self.mapping_context and not self.indention)
        self.increase_indent(flow=True)                  
        self.state = self.expect_first_block_sequence_item

@DanyC97
Copy link

DanyC97 commented May 15, 2019

@ingydotnet @perlpunk any chance you can shed some light and i may be able to fire a PR ?

@perlpunk
Copy link
Member

perlpunk commented Jun 2, 2019

Well, I wouldn't call that behaviour incorrect.
I guess it's a matter of taste, and I can find arguments proving that it's consistent.
What I'm also missing in this issue is the expected correct behaviour.

Let's look at both examples:

--- # spaces = 4
list_of_dict_attr:
-   attr1: value1
    attr2: value2
    attr3:
    - item1
    - item2
single_attr: value1
--- # spaces = 2
list_of_dict_attr:
- attr1: value1
  attr2: value2
  attr3:
  - item1
  - item2
single_attr: value1

The top level mapping has an indentation of zero (0 * spaces).
The value for list_of_dict_attr, the sequence, also has an indentation of zero, because PyYAML chooses zero-indented sequences always. That's why the dashes have no indentation in both cases. If it chooses zero-indentation, it simply does not depend on the number of spaces you configured.

The value of the first sequence item, the mapping attr1: ..., has an indentation of 1 * spaces (respectively 4 or 2).
The sequence under attr3 is zero-indented again, so 1 * spaces. The items of this sequence are on the same line, so they don't get any indentation.

I assume you would expect this instead?

--- # spaces = 4
list_of_dict_attr:
  - attr1: value1
    attr2: value2
    attr3:
      - item1
      - item2
single_attr: value1

@kislyuk
Copy link

kislyuk commented Jun 24, 2019

I can't speak for @DanyC97 but in my opinion yes, that last snippet is what is expected when passing spaces=4.

@pbasista
Copy link

From my point of view, the most widely-accepted indentation style for sequences is the one used multiple times in the official YAML specification. For instance, in section 2.1, example 2.3 looks like this:

american:
  - Boston Red Sox
  - Detroit Tigers
  - New York Yankees
national:
  - New York Mets
  - Chicago Cubs
  - Atlanta Braves

The question is whether tools like pyyaml should render sequences in such a way for indentation of size 4 or for indentation of size 2.

I would argue that it seems incorrect to render sequences in such a way for indentation of size 4, because other items would visually appear to be indented more:

mapping:
    one: 1
    two: 2
list:
  - 1
  - 2

Therefore, I think that it is more appropriate to render sequences in such a way for indentation of size 2:

mapping:
  one: 1
  two: 2
list:
  - 1
  - 2

That being said, someone may prefer to not indent sequence items to a level that is visually similar to the indentation level of the other items. That is a fair requirement, but in order to fully support it, there would have to be a separate configuration option for indentation size of sequences.

@PeterDaveHello
Copy link

PeterDaveHello commented Jul 1, 2019

I agree with @pbasista about what the output should look like, that'll be the same behavior that yamllint using, and maybe a separate configuration option would be a solution for both people want/like it or not.

I'm currently facing yaml file generated by pyyaml not being accepted by yamllint because of the indent of the lists.

@perlpunk
Copy link
Member

perlpunk commented Jul 1, 2019

there would have to be a separate configuration option for indentation size of sequences.

From my point of view that would be the best, because IMHO currently block sequences are simple not indented at all (at least when they are a value of a block mapping), independent of the indent option.

@cortex3
Copy link

cortex3 commented Oct 25, 2019

Is there any progress on this?
Right now the workaround here is working for me https://stackoverflow.com/questions/25108581/python-yaml-dump-bad-indentation

@jagibson
Copy link

It seems the spec varies the output. The Preview section shows sequences indented from the key.

Example 2.3. Mapping Scalars to Sequences
(ball clubs in each league)

american:
  - Boston Red Sox
  - Detroit Tigers
  - New York Yankees
national:
  - New York Mets
  - Chicago Cubs
  - Atlanta Braves

However the Failsafe Schema is indeed what pyyaml is doing:

10.1. Failsafe Schema
The failsafe schema is guaranteed to work with any YAML document. It is therefore the recommended schema for generic YAML tools. A YAML processor should therefore support this schema, at least as an option.
...
10.1.1.2. Generic Sequence
URI:
tag:yaml.org,2002:seq

Kind:
Sequence.

Definition:
Represents a collection indexed by sequential integers starting with zero. Example bindings to native types include Perl’s array, Python’s list or tuple, and Java’s array or Vector.

Example 10.2. !!seq Examples

Block style: !!seq
- Clark Evans
- Ingy döt Net
- Oren Ben-Kiki

Flow style: !!seq [ Clark Evans, Ingy döt Net, Oren Ben-Kiki ]

Personally I prefer the indented format and it would be nice if pyyaml supported it as an option but the code isn't doing anything wrong without the indents even if yamllint disagrees.

@unexceptable
Copy link

unexceptable commented Aug 7, 2020

Just ran into this myself in some example config generation I am doing, and it makes some of my exports rather weird.

The indented list structure does seem more common, and it would be nice if pyyaml supported it.

Edit: I've now also temporarily solved it with https://stackoverflow.com/questions/25108581/python-yaml-dump-bad-indentation.

@kaysond
Copy link

kaysond commented Oct 19, 2020

Also running into this via ansible. It would be nice if the indentation were consistent.

@andreif
Copy link

andreif commented Jan 23, 2021

The workaround mentioned above:

class Dumper(yaml.Dumper):
    def increase_indent(self, flow=False, *args, **kwargs):
        return super().increase_indent(flow=flow, indentless=False)

print(yaml.dump(data, Dumper=Dumper))
Gateways:
  - 14
  - 4
  - 18

@yajo
Copy link

yajo commented Jan 26, 2021

I just use prettier as a pre-commit hook and it takes care of making yaml look good.

@Jean-Daniel
Copy link

The workaround mentioned above:

class Dumper(yaml.Dumper):
    def increase_indent(self, flow=False, *args, **kwargs):
        return super().increase_indent(flow=flow, indentless=False)

print(yaml.dump(data, Dumper=Dumper))

Unfortunately, it does not works with CDumper. And when working with a lots of yaml or with big yaml, I'd rather have no indentation than having a "good looking" one but much much slower generator.

@pillarsdotnet
Copy link

I just use prettier as a pre-commit hook and it takes care of making yaml look good.

I tried using prettier, but it made changes that both I and YamlLint disagreed with.

openstack-mirroring pushed a commit to openstack-archive/tripleo-validations that referenced this issue Jul 5, 2021
The validation_init role auto generates the molecule.yaml file when
adding a new role. But, due to [1][2], the to_nice_yaml filter is not
correctly indenting lists and will break the indentation every time a
new validation will be created.

This patch adds this file to the yamllint ignore list and provides a new
molecule.yaml sorted by job names as the validation_init role will
generate in a follow up patch.

Moreover, this patch enables the oslo_config_validator molecule job in
the check/gate as it was not added in the initial patch[3].

[1] ansible/ansible#48865
[2] yaml/pyyaml#234
[3] https://review.opendev.org/c/openstack/tripleo-validations/+/789698

Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
Change-Id: Iac63dc6f161a0e885110a246b68001522a6f62e3
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Jul 5, 2021
* Update tripleo-validations from branch 'master'
  to 7c35cd1fb7d641c10b7575e1dd354aea4e182dd9
  - Merge "Add zuul.d/molecule.yaml file back in the yamllint ignore list"
  - Add zuul.d/molecule.yaml file back in the yamllint ignore list
    
    The validation_init role auto generates the molecule.yaml file when
    adding a new role. But, due to [1][2], the to_nice_yaml filter is not
    correctly indenting lists and will break the indentation every time a
    new validation will be created.
    
    This patch adds this file to the yamllint ignore list and provides a new
    molecule.yaml sorted by job names as the validation_init role will
    generate in a follow up patch.
    
    Moreover, this patch enables the oslo_config_validator molecule job in
    the check/gate as it was not added in the initial patch[3].
    
    [1] ansible/ansible#48865
    [2] yaml/pyyaml#234
    [3] https://review.opendev.org/c/openstack/tripleo-validations/+/789698
    
    Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
    Change-Id: Iac63dc6f161a0e885110a246b68001522a6f62e3
openstack-mirroring pushed a commit to openstack-archive/tripleo-validations that referenced this issue Jul 5, 2021
This patch adds a mechanism to keep the job names in
zuul.d/molecule.yaml sorted. Therefore, the to_nice_yaml filter is not
indenting lists correctly, due to [1][2].

[1] ansible/ansible#48865
[2] yaml/pyyaml#234

Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
Change-Id: Ia8f787631c55fabb3a097919971d17b93711d4b7
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Jul 5, 2021
* Update tripleo-validations from branch 'master'
  to ea3931c0f65278aefc784a633de99d0f225414b5
  - Merge "Sort jobs in molecule.yaml file when adding new role"
  - Sort jobs in molecule.yaml file when adding new role
    
    This patch adds a mechanism to keep the job names in
    zuul.d/molecule.yaml sorted. Therefore, the to_nice_yaml filter is not
    indenting lists correctly, due to [1][2].
    
    [1] ansible/ansible#48865
    [2] yaml/pyyaml#234
    
    Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
    Change-Id: Ia8f787631c55fabb3a097919971d17b93711d4b7
openstack-mirroring pushed a commit to openstack-archive/tripleo-validations that referenced this issue Jul 6, 2021
The validation_init role auto generates the molecule.yaml file when
adding a new role. But, due to [1][2], the to_nice_yaml filter is not
correctly indenting lists and will break the indentation every time a
new validation will be created.

This patch adds this file to the yamllint ignore list and provides a new
molecule.yaml sorted by job names as the validation_init role will
generate in a follow up patch.

Moreover, this patch enables the oslo_config_validator molecule job in
the check/gate as it was not added in the initial patch[3].

[1] ansible/ansible#48865
[2] yaml/pyyaml#234
[3] https://review.opendev.org/c/openstack/tripleo-validations/+/789698

Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
Change-Id: Iac63dc6f161a0e885110a246b68001522a6f62e3
(cherry picked from commit 41dcda8)
openstack-mirroring pushed a commit to openstack-archive/tripleo-validations that referenced this issue Jul 6, 2021
This patch adds a mechanism to keep the job names in
zuul.d/molecule.yaml sorted. Therefore, the to_nice_yaml filter is not
indenting lists correctly, due to [1][2].

[1] ansible/ansible#48865
[2] yaml/pyyaml#234

Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
Change-Id: Ia8f787631c55fabb3a097919971d17b93711d4b7
(cherry picked from commit ae59abe)
openstack-mirroring pushed a commit to openstack-archive/tripleo-validations that referenced this issue Jul 16, 2021
The validation_init role auto generates the molecule.yaml file when
adding a new role. But, due to [1][2], the to_nice_yaml filter is not
correctly indenting lists and will break the indentation every time a
new validation will be created.

This patch adds this file to the yamllint ignore list and provides a new
molecule.yaml sorted by job names as the validation_init role will
generate in a follow up patch.

Moreover, this patch enables the oslo_config_validator molecule job in
the check/gate as it was not added in the initial patch[3].

[1] ansible/ansible#48865
[2] yaml/pyyaml#234
[3] https://review.opendev.org/c/openstack/tripleo-validations/+/789698

Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
Change-Id: Iac63dc6f161a0e885110a246b68001522a6f62e3
(cherry picked from commit 41dcda8)
(cherry picked from commit f798c56)
openstack-mirroring pushed a commit to openstack-archive/tripleo-validations that referenced this issue Jul 16, 2021
This patch adds a mechanism to keep the job names in
zuul.d/molecule.yaml sorted. Therefore, the to_nice_yaml filter is not
indenting lists correctly, due to [1][2].

[1] ansible/ansible#48865
[2] yaml/pyyaml#234

Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
Change-Id: Ia8f787631c55fabb3a097919971d17b93711d4b7
(cherry picked from commit ae59abe)
(cherry picked from commit cc29577)
openstack-mirroring pushed a commit to openstack-archive/tripleo-validations that referenced this issue Jul 31, 2021
The validation_init role auto generates the molecule.yaml file when
adding a new role. But, due to [1][2], the to_nice_yaml filter is not
correctly indenting lists and will break the indentation every time a
new validation will be created.

This patch adds this file to the yamllint ignore list and provides a new
molecule.yaml sorted by job names as the validation_init role will
generate in a follow up patch.

Moreover, this patch enables the oslo_config_validator molecule job in
the check/gate as it was not added in the initial patch[3].

[1] ansible/ansible#48865
[2] yaml/pyyaml#234
[3] https://review.opendev.org/c/openstack/tripleo-validations/+/789698

Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
Change-Id: Iac63dc6f161a0e885110a246b68001522a6f62e3
(cherry picked from commit 41dcda8)
(cherry picked from commit f798c56)
(cherry picked from commit 04c0871)
openstack-mirroring pushed a commit to openstack-archive/tripleo-validations that referenced this issue Sep 1, 2021
This patch adds a mechanism to keep the job names in
zuul.d/molecule.yaml sorted. Therefore, the to_nice_yaml filter is not
indenting lists correctly, due to [1][2].

[1] ansible/ansible#48865
[2] yaml/pyyaml#234

Signed-off-by: Gael Chamoulaud (Strider) <gchamoul@redhat.com>
Change-Id: Ia8f787631c55fabb3a097919971d17b93711d4b7
(cherry picked from commit ae59abe)
(cherry picked from commit cc29577)
(cherry picked from commit da80177)
@xkungfu
Copy link

xkungfu commented Jan 13, 2022

another year 2022 coming, is there an easy way to resolve this issue?

@Acidherr
Copy link

Acidherr commented May 20, 2022

another year 2022 coming, is there an easy way to resolve this issue?

#234 (comment)

This worked for me. It is quite easy to implement also.

maage added a commit to maage/scap-security-guide that referenced this issue Mar 21, 2023
See: yaml/pyyaml#234 (comment)

At least in Fedora 36 / 37 increase_indent does not work with CDumper.

python3-pyyaml-6.0-3.fc36.x86_64
python3-pyyaml-6.0-5.fc37.x86_64

Python 3.10.6 (main, Aug  2 2022, 00:00:00) [GCC 12.1.1 20220507 (Red Hat 12.1.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import yaml
>>>
>>> try:
...     from yaml import CDumper as yaml_Dumper
... except ImportError:
...     from yaml import Dumper as yaml_Dumper
...
>>> class TestDumper(yaml_Dumper):
...     def increase_indent(self, flow=False, indentless=False):
...         return super(yaml_Dumper, self).increase_indent(flow, False)
...
>>> yaml.dump({"a":[1]}, None, TestDumper)
'a:\n- 1\n'
>>>
>>> unformatted_yaml = yaml.dump({"a":[1]}, None, TestDumper)
>>> if unformatted_yaml == 'a:\n- 1\n':
...         from yaml import Dumper as yaml_Dumper
...
>>> class TestDumper(yaml_Dumper):
...     def increase_indent(self, flow=False, indentless=False):
...         return super(yaml_Dumper, self).increase_indent(flow, False)
...
>>> yaml.dump({"a":[1]}, None, TestDumper)
'a:\n  - 1\n'
@terwer
Copy link

terwer commented Apr 4, 2023

Sill not work, will this be fixed?

my code

def test_demo(self):
    data = {
        "name": "John",
        "age": 30,
        "city": "New York",
        "haha": ["aaaa", "bbbb"]
    }

    # 设置 indent 和 default_flow_style 参数
    output = yaml.dump(data, Dumper=Dumper,  sort_keys=False,indent=2)
    print(output)

result is

age: 30
city: New York
haha:
- aaaa
- bbbb

what I expected is

age: 30
city: New York
haha:
  - aaaa
  - bbbb

@terwer
Copy link

terwer commented Apr 4, 2023

Found a temp solution

https://stackoverflow.com/a/39681672/4037224

@pkit
Copy link

pkit commented Jul 17, 2023

@Acidherr

#234 (comment)

This worked for me. It is quite easy to implement also.

What in the words indent=4 is so hard to understand?
No it doesn't work with indent=4, in 2023.

@kislyuk
Copy link

kislyuk commented Jul 17, 2023

@pkit I know you're frustrated about the slow progress on this issue - many of us are - but please do not take your frustration out on your fellow commenters and contributors. By doing so you reduce trust and diminish the quality of all open source projects related to this one.

moritzrp added a commit to moritzrp/Automated-Ansible-Role-Documentation that referenced this issue Oct 6, 2024
This addresses yaml/pyyaml#234. PyYAML is
currently ignoring the indent for lists when dumping. Since ansible-lint
expects the indent, this is problematic.
@tpcgold
Copy link

tpcgold commented Dec 3, 2024

I just discovered this issue, and I'm running into it. It is ridiculous that the team around pyyaml hasn't provided a fix for this since 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests