cmp argument parser rewrite
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 4/5
- Thời gian dự kiến
- 3-5 ngày
- Mức phù hợp với người mới
- 35/100
Hướng nghiên cứu
Bắt đầu bằng việc xem xét src/cmp.rs và phiên bản được đề xuất trong fork được liên kết, tập trung vào việc phân tích đối số, Params, ParamsParseError, xử lý help/version và các dạng tùy chọn được liệt kê. Xem xét các kiểm thử cmp hiện có và các kiểm thử được thêm trong fork. Được xem là hoàn tất khi các thay đổi parser và các quyết định về public API được thống nhất, các tùy chọn và lỗi được hỗ trợ khớp với hành vi dự kiến, và các kiểm thử đều vượt qua.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
Hello everyone,
I was checking the functionality of the cmp.rs and found some issues around not working options/arguments.
I did a lot of testing regarding the parser and found the following rules.
/// Parses the command line arguments. \
/// Since cmp is called from diffutils, the first argument must always be "cmp".
///
/// The following checks require more extensive checks than a simple compare.
/// These are all identical and make parsing extensive:
/// - cmp file_1 file_2 -b -l -n 50
/// - cmp file_1 file_2 -b -l -n50
/// - cmp file_1 file_2 -bl -n50
/// - cmp file_1 file_2 -bln 50
/// - cmp file_1 file_2 -bln50
/// - cmp file_1 file_2 --print-bytes --verbose --bytes 50
/// - cmp file_1 file_2 --print-bytes --verbose --bytes=50
/// - cmp file_1 file_2 --p --verb --by 50
/// - cmp file_1 file_2 --p --verb --by=50
///
/// and many more rules.
A number of them are not working in the current cmp.rs version.
After I started to fix some of the issues, I started refactoring the whole parser.
- Separation of concerns:
- I moved all code regarding parsing into to Params struct.
- The parser now only returns enums (Params or InfoText for Version and Help) for the OK case or a specified Error.
- All errors have been moved into an Error Enum ParamsParseError which handles all output in Display.
- This allows a centralized maintenance of errors and error messages. The caller can work with the error enum more easily.
- Renaming of the Param properties.
- from and to replaced with file_1 and file_2, as these are the parameters.
- max_bytes with bytes_limit
- quiet with silent as s is the short code for it
- skip_a with ignore_initial_bytes_file_1, so there is a connection between options and the property name
- The bytes variables where size usize, thus limiting the readable bytes on 32-bit systems. GNU cmp is compliles with
Large File Support and allows i64 values. I changed it to u64 with a Feature to change it to u128. - bytes-limit: The tests set the bytes-limit to MAX:usize if a very large number is encountered. This is incorrect,
GNU cmp gives an error message. - Corrected parser, e.g. -bln50KiB is now allowed or --ig for --ignore-initial
- Correct output of error messages, Help text is missing. New error messages like in GNU cmp.
- Added text for --version and --help
- Tests
- Added new tests for the above valid options.
- Changed tests where bytes where set to MAX.
- Tests not can be written with simple String "cmp foo bar -n 50"
Questions:
- Params executable: What is the reason to store this value? It is the argument 'cmp' which is used to start the program.
I don't see how this can ever change and it requires a lot of extra code to pass it to the error messages.
const APP_NAME: &str = 'cmp'; would be sufficient, or am I missing something here? I would like to remove it. - GNU cmp behaves very odd when passing '--'. Try 'cmp -- file_1 --help'. Why whould --help be a file?
I implemented an error message instead, as the usage is probably not intended.
Is this an issue, as it would not behave 100% like GNU cmp. Try 'cmp -- file_1 --help'. Why whould --help be a file? - The error messages are slightly changed, e.g.
- 'option requires an argument -- 'n'' when only passing -n, but
- 'option '--bytes' requires and argument', which is nicer to read.
I do not differentiate here and print: - 'option '--bytes' ('-n') requires an argument'
- Params is generally public and could be used elsewhere, e.g. the number conversion. Should I keep it public?
Before submitting a PR I would like to have some feedback on this and the refactoring.
Check out the new version at my fork.
- Ngôn ngữ chính
- Rust
- Star
- 276
- Fork
- 39
- Merge trung bình
- 4 ngày 12 giờ
- Pull request đã merge (30 ngày)
- 3
Hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của uutils/diffutils
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 70/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 85/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 72/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 68/100
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 45/100
Tất cả issue của uutils/diffutils
Issue tương tự
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 75/100
TheLarkInn/aipm#2413 ·
-
documentation
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 90/100
alexgorbatchev/simple-ptt#15 ·
-
tooling
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 75/100
-
todo:ticket
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 70/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 75/100
taikoxyz/taiko-mono#22168 · 1 bình luận ·