Visitar URL original
`file-contents-sorter` Not working same way everytime · Issue #794 · pre-commit/pre-commit-hooks · GitHub
Skip to content

file-contents-sorter Not working same way everytime #794

Description

@teksturi

I get strange issue that file-contents-sorter hook does not work same way everytime. I made little test case which trickers the issue. If i run this 10 times it will sometimes pass and sometimes not. I did not have time to look what problem might be.

Add this to file_contents_sorter_test.py

(
    b'pre\nPre\n',
    ['--unique', '--ignore-case'],
    PASS,
    b'pre\nPre\n',
),

And run it with
for run in {1..10}; do pytest tests/file_contents_sorter_test.py; done
This gives me that about 30% pass and 70% fails

Activity

  1. asottile commented on Jul 29, 2022

    @asottile
    Member

    yeah this set call breaks unique sorting -- it should probably just forbid the combination of unique and ignore case since the result is undefined https://github.com/pre-commit/pre-commit-hooks/blob/v4.3.0/pre_commit_hooks/file_contents_sorter.py#L36

  2. renegaderyu commented on Oct 19, 2022

    @renegaderyu

    @asottile please review if you have time. I'm hoping this PR is simple enough and goes with the spirit of forbidding the combinations of options as you mentioned. Also, I'd appreciate if you could label w/ hacktoberfest-accepted so I can get a tree planted, thanks.

  3. asottile commented on Oct 19, 2022

    @asottile
    Member

    I'm not going to review something which doesn't pass tests

  4. renegaderyu commented on Oct 20, 2022

    @renegaderyu

    @asottile Apologies for not seeing the failing tests before asking. I think its ready now.

  5. XuehaiPan commented on Mar 27, 2023

    @XuehaiPan

    yeah this set call breaks unique sorting -- it should probably just forbid the combination of unique and ignore case since the result is undefined v4.3.0/pre_commit_hooks/file_contents_sorter.py#L36

    Could we respect the original word case if the uncased words are equal? For example, let Pre always proceed pre because it's upper case. This can be easily implemented with tuple-based sort keys.

    def key(s):
        return (s.lower(), s)
    
    ret = sorted(set(word_list), key=key)

    Another option is to respect the original order (stable sort). This can use an "OrderedSet" rather than set.

    def unique(iterable):
        return list(OrderedDict.fromkeys(iterable))  # can be replaced with `dict` for Python 3.7+

    Both approaches I list above are deterministic.

  6. asottile commented on Mar 27, 2023

    @asottile
    Member
  7. Childcity commented on Sep 14, 2023

    @Childcity
  8. asottile commented on Sep 14, 2023

    @asottile
  9. Childcity commented on Sep 14, 2023

    @Childcity
  10. asottile commented on Sep 14, 2023

    @asottile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions