cmp argument parser rewrite
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 35/100
Direzione di ricerca
Inizia esaminando src/cmp.rs e la versione proposta nel fork collegato, concentrandoti sull’analisi degli argomenti, Params, ParamsParseError, la gestione di help/version e le forme delle opzioni elencate. Esamina i test esistenti di cmp e i test aggiunti nel fork. Il lavoro è completato quando le modifiche al parser e le decisioni sull’API pubblica sono concordate, le opzioni e gli errori supportati corrispondono al comportamento previsto e i test hanno esito positivo.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
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.
- Lingua principale
- Rust
- Stelle
- 276
- Fork
- 39
- Merge medio
- 3h 27m
- PR unite (30g)
- 3
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di uutils/diffutils
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 70/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 64/100
Tutte le issue di uutils/diffutils
Issue simili
-
Replayed reasoning items send "content": null, which the Responses API schema does not permit Apertabug CLI custom-model
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
rust-bitcoin/rust-bitcoin#6930 · 1 commento ·
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
fulcrumgenomics/ferro-hgvs#2251 ·
-
A-allocators A-docs C-enhancement T-libs
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100