comm: implement the ordering check #7144
Conversation
|
GNU testsuite comparison: |
sylvestre
force-pushed
the
comm2
branch
3 times, most recently
from
df0ed16 to
9499902
Compare
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
There was a problem hiding this comment.
Are the tests check_order, defaultcheck_order, and nocheck_order still needed? They are currently marked as "ignore".
There was a problem hiding this comment.
I missed them
ok if I fix that in a following PR?
| @@ -404,7 +409,7 @@ fn unintuitive_default_behavior_1() { | |||
| scene | |||
| .ucmd() | |||
| .args(&["defaultcheck_unintuitive_1", "defaultcheck_unintuitive_2"]) | |||
| .succeeds() | |||
| .fails() | |||
There was a problem hiding this comment.
Here it might be useful to have a comment that the behavior differs from GNU comm, which doesn't fail.
There was a problem hiding this comment.
are you sure? it fails for me
$ echo "m\nh\nn\no\nc\np\n" > defaultcheck_unintuitive_1
$ echo "m\nh\nn\no\np\n" > defaultcheck_unintuitive_2
$ /usr/bin/comm defaultcheck_unintuitive_1 defaultcheck_unintuitive_2
$ echo $?
1
There was a problem hiding this comment.
I think the reason it fails for you is that echo adds a trailing newline by default.
There was a problem hiding this comment.
oh, right. Comment added :)
| } | ||
|
|
||
| self.last_line = current_line.to_vec(); | ||
| is_ordered || !self.check_order |
There was a problem hiding this comment.
I'm struggling with the || !self.check_order part. I think this functionality should be somewhere else. It feels illogical that something that's not ordered suddenly can become ordered ;-)
There was a problem hiding this comment.
not sure how to fix that?! sorry
sylvestre
force-pushed
the
comm2
branch
2 times, most recently
from
166b120 to
85f2604
Compare
|
GNU testsuite comparison: |
A few comments: * skip if the two args are pointing to the same file * skip if the same content in the two files * implement --check-order * implement --nocheck-order * output the right things on stderr Should pass: tests/misc/comm
|
GNU testsuite comparison: |
Well done :) |
A few comments:
Should pass: tests/misc/comm