Apply sort order to files in line and grid outputs #756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi, do you think you can fix the merge conflicts so that I can merge your PR? |
whonore
force-pushed
the
fix-sort-arguments
branch
from
577986f to
c8ed693
Compare
|
Done. Does it look ok now? |
|
Your changes makes some existing extended tests fail, and the ones you added don’t work either on my computer. Also you mistyped failing testsI don’t know why some tests started failing, but I believe the one you introduced are failing because the output you got was from executing exa in a different directory or with an alias. |
|
Sorry about that, I remember I was having some trouble running the extended tests locally so I was hoping the CI could do it for me. I'll try to fix them soon. |
|
Ok, I believe I fixed the new tests I added. The problem was As for the other tests that I didn't write, the problem is they're relying on bash's glob sort order, but my change runs everything through
|
|
Hi, The reason you discovered that the extended tests are failing is correct — those tests require a large list of files with directories, and while the purpose of those tests isn't to test the arguments' sort order, it inadvertently tested it anyway. (At least it was commented.) I'm actually surprised that there was no existing text that explicitly tests this behaviour. Maybe the failing tests were the tests, and their purpose got lost to time. Anyway, I'm going to merge this PR and go with option (2), changing the behaviour, then fix the tests to support the new defined behaviour. I thought about it for a little while and realised that no matter which behaviour we pick (whether to sort or to not sort), exa might do something unexpected:
We could try to be smart and apply some heuristic, but I think that introduces more problems than it solves. Plus, I checked the GNU and BSD Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #404