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

Retrieve aliases not for keys #36

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

Conversation

flavono123
Copy link

Correct the detecting wrong key in such following YAML and test:

$ cat alias_dup_values.yaml
---
foo: &foo
bar:
  <<: *foo
  key1: val
  key2: val
  key3: val
$ yamllint alias_dup_values.yaml
Checking 1 files
alias_dup_values.yaml
  The same key is defined more than once: bar.val
YAML lint found 1 errors

And refactor to reduce codes

@@ -151,28 +151,30 @@ def parse(psych_parse_data)
def parse_recurse(psych_parse_data, is_sequence = false)
is_key = false
psych_parse_data.children.each do |n|
case n.class.to_s
when 'Psych::Nodes::Scalar'
case n
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't this be n.class?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine, though it's a mildly surprising idiom. It's designed to work this way: case...when in Ruby compares with ===, and Module#=== is defined to check whether the object is an instance of the class.

@shortdudey123
Copy link
Owner

Thanks for the fix!
After #39 is merged, I will have you rebase on master to verify the tests pass. Also, can you add a line to the CHANGELOG in the unreleased section following the existing format?

This was referenced Sep 23, 2021
@@ -0,0 +1,6 @@
---
Copy link
Owner

Choose a reason for hiding this comment

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

This file needs to be added to the rakefile under the yamllint_exclude_paths test.

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.

3 participants