Skip to content

Conversation

yedayak
Copy link
Collaborator

@yedayak yedayak commented Oct 13, 2025

Also fix all current ones

Also fix all current ones
Copy link
Collaborator

Choose a reason for hiding this comment

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

These trailing white spaces in compgen/t{1..3}.txt are produced by Bash's declare. However, the files compgen/t{1..3}.txt don't seem to be used by tests. They were added by commit c9c98da, but I'm not sure the reason that these files are added. At least, even if I remove those files, the following tests still pass:

$ pytest test/t/test_screen.py
$ pytest test/t/test_compgen.py
$ pytest test/t/unit

Can we remove these files?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is what ip addr produces. Shouldn't we use the output of ip addr as is for testing purposes? For example, to ensure that the completion generation still works with trailing white spaces. shared/bin/ifconfig for ifconfig -a is similar.

Copy link
Owner

@scop scop left a comment

Choose a reason for hiding this comment

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

I think I'd rather not do this. At least some of the affected files are intentional as-is copies of outputs etc that are better left as-is. For cases where it "matters" more we already have shfmt taking care of it for shell code, and ruff for python.

@yedayak
Copy link
Collaborator Author

yedayak commented Oct 14, 2025

I think I'd rather not do this. At least some of the affected files are intentional as-is copies of outputs etc that are better left as-is. For cases where it "matters" more we already have shfmt taking care of it for shell code, and ruff for python.

Yeah makes sense, thanks for the review

@yedayak yedayak closed this Oct 14, 2025
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