-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
tr
: Expanding expansion module
#2502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tr
: Expanding expansion module
#2502
Conversation
f2a228d
to
7a03664
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! This indeed brings us much closer to full compatibility. I've left some suggestions, mostly for cleaning up some of the nom
code, but nothing really major.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more tiny suggestions. You've made some nice changes since I last looked at this. The overall design looks very good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some errors abuot the spelling. You can disable the spell checker with a comment on top of the file like:
//spell-checker: ignore coreutil |
Except for the compile error for char::from_utf8
, all compile errors will soon go away with our new minimum rust version 1.51, so you can ignore those.
Could you please fix the clippy warnings?
|
I am going to put it as draft again. Trying to pass GNU test. |
I have since added all the GNU tests as rust tests --- Some of them are quite confusing to me so please take a look at it. I try to annotate the weird ones with Aside from that, it passes all of them. So check if they are correct or not. I had to do some transformation to the GNU tests i.e. converting |
a few failure errors
|
I guess you saw also:
|
Oh in windows... Edit: This is something windows-specific that I simply don't understand. |
Some discussion on the current problem clap-rs/clap#2654 |
Hopefully will be feature parity with GNU `tr`. Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com> Implemented a bit of new expansion module Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com> Implemented delete operation Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com> Partially implemented delete operation Will go through translate next. Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com> Fix formatting... Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com> Implemented translation feature Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
…implement-expansion
…implement-expansion
…implement-expansion
…na-tr-reimplement-expansion
…eutils into hbina-tr-reimplement-expansion
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Ariffin <hanif.ariffin.4326@gmail.com>
Signed-off-by: Hanif Ariffin <hanif.ariffin.4326@gmail.com>
…implement-expansion
…eutils into hbina-tr-reimplement-expansion
Something like this @sylvestre ? |
Do you know why the GNU test results aren't changing? |
Prolly cause
Something wrong with the test environment?
|
Hopefully, #2753 will improve that :) |
106658e
to
342a7ad
Compare
342a7ad
to
b51a6e8
Compare
I've added some final changes and I think this is ready to be merged now! |
Yipes! That merged more than 15 sub-merge commits. |
Ah sorry! I didn't look at the history. Will keep that in mind in the future! |
Extending the functionality of
tr
to possibly be feature-parity with GNUtr
.