feat: add the ability to ignored keys while sorting

[fernandezcuesta]

Hi, the use case here is to allow nested maps such as:

versions:
  - name: v1alpha1
    capabilities: [ ... ]
    customField: ""
  - name: v1alpha2
    customField: ""

not to be sorted, since there are times when we consider a primary/merge-key useful to remain wherever we define it (usually in 1st place)

[coveralls]

coverage: 99.825%. remained the same
when pulling 1efd875 on fernandezcuesta:master
into e118296 on adrienverge:master.

[adrienverge]

Hello J.M., thanks for contributing!

The idea looks good. I propose these modifications:

• Name the new option ignored-keys for explicitness (if we add other "ignored" options in the future).
• Accept regexes instead of fixed strings, for more powerful usage. See for example the rule quoted-strings with its options extra-required and extra-allowed.

Also, can you include more tests? Especially for more than 1 ignored-keys, and with the regex feature we should also check it, e.g. name, aaa name and name zzz for ignored-keys: ['name'] vs. ignored-keys: ['^name$'].

Finally, you need to add a .. rubric:: Options in documentation (see other rules for examples).

[adrienverge]

Don't delete this line

[fearphage]

I'm very motivated to get this merged and I see @fernandezcuesta hasn't responded in over a month. Would any be opposed to me making the reviewer comment changes in a new PR? /cc @adrienverge

FYI my use case is GitHub Actions. Sorting some of the keys there makes things more confusing/harder to maintain for humans (read: bad).

[fernandezcuesta]

I'll do it, apologies for the long delay

[adrienverge]

Hello Jesús,

Thanks for the changes.

While trying to improve test coverage (see my comment below) I found a bug with this implementation: with ignored-keys: ["n(a|o)me", "^b"], it reports the line boat as an error, while it shouldn't:

a:
b:
c:
name: ignored
first-name: ignored
nome: ignored
gnomes: ignored
d:
e:
boat: ignored
.boat: ERROR
call: ERROR
f:
g:

This is because you keep adding the ignored keys to the list of previous keys to compare (context['stack'][-1].keys).

Instead of, I think your commit should simply do:

@@ -116,7 +142,9 @@ def check(conf, token, prev, next, nextnext, context):
           isinstance(next, yaml.ScalarToken)):
         # This check is done because KeyTokens can be found inside flow
         # sequences... strange, but allowed.
-        if len(context['stack']) > 0 and context['stack'][-1].type == MAP:
+        if (len(context['stack']) > 0 and context['stack'][-1].type == MAP and
+                not any(re.search(r, next.value)
+                        for r in conf['ignored-keys'])):
             if any(strcoll(next.value, key) < 0
                    for key in context['stack'][-1].keys):
                 yield LintProblem(

[adrienverge]

@fernandezcuesta it looks like you didn't check all the comments from my last review.

Would any be opposed to me making the reviewer comment changes in a new PR? /cc @adrienverge

@fearphage feel free if needed!

[adrienverge]

No need to change this; and also please don't change my code proposals to use Black style: the whole codebase doesn't.

[fernandezcuesta]

sorry for that

[fearphage]

@adrienverge How are things looking now?

[fearphage]

@adrienverge Just checking in again to see if there's anything preventing this PR from being merged.

[adrienverge]

@fearphage I'm sorry, please excuse the long delay…

It looks mergable now that my comments are addressed. I just reworded the documentation to be clearer (the previous version looked like a copy-paste of quoted-strings docs, but the logic isn't the same here), and squashed everything to follow the contributing rules and have readable commits.

🤔 When I pushed these changes to [email protected]:fernandezcuesta/yamllint.git master, GitHub closed the pull request, and I can't push on it (permission denied). I guess it's because this PR was opened from a branch named master. I opened a new pull request to continue this: #714.