-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
tsort: gnu misc tsort.pl #9289
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
tsort: gnu misc tsort.pl #9289
Conversation
…raph topologies - Introduce new test cases for cycle loops in file inputs, including extra nodes and multiple loops - Add tests for POSIX graph examples and linear tree graphs to validate topological sorting - Include tests for error handling on odd token counts and multiple file inputs - Ensures robustness and correctness of tsort implementation across various edge cases and standard scenarios
…ne literals in tsort test file - Reformatted TSORT_LOOP_STDERR_AC and TSORT_UNEXPECTED_ARG_ERROR constants for improved code readability and consistency, with no change in string content or functionality. The multi-line format was merged into single lines to align with potential linting rules or style preferences for string literals in tests. This refactoring enhances maintainability without affecting test logic.
- Added `ArgAction` import and a new hidden `-w`/`--warn` argument to the tsort command for future warning features - Modified iteration of successor names to use `.into_iter().rev()` in the topological sorting algorithm to process nodes in reverse order, ensuring more stable and predictable output sequence - Refactored Clap error localization in `clap_localization.rs` to use `print_prefixed_error()` method instead of direct `eprintln!` calls, improving consistency in error message formatting across the application
Update expected error outputs in uniq tests to match the new format where error messages are prefixed with "uniq: ". This ensures test cases align with the updated error message formatting in the utility, providing clearer error identification by including the program name at the start of each error message. Changes affect multiple test assertions for invalid options and incompatible argument combinations.
The expected error message now starts with "chroot: " to match the updated output format in the chroot utility. This ensures the test accurately reflects the command's behavior.
…d format Remove "chroot: " prefix from expected error output, aligning the test with the updated stderr format that omits utility name redundancy in error messages.
Remove the 'uniq: ' prefix from expected error messages in test cases to match the updated output format of the uniq utility, ensuring tests pass with the current implementation. This change affects multiple GNU compatibility tests for invalid options and argument conflicts.
…t 'comm: ' prefix The stderr assertion in test_comm_arg_error was expecting an error message prefixed with "comm: ", but the actual command output does not include this prefix. This update fixes the test to align with the real behavior, ensuring the test passes correctly.
… stderr output in ErrorFormatter Replace the call to self.print_prefixed_error with direct eprintln for printing unexpected argument errors, and add an additional blank line for better formatting and readability in error messages. This change aims to simplify the output process and ensure consistent error presentation in the clap localization module.
…irect eprintln for cleaner output Modified error handling in clap_localization.rs to eliminate the utility name prefix by replacing self.print_prefixed_error calls with direct eprintln! invocations. This simplifies the codebase and changes error message formatting to display clap errors without the preceding util name. Removed the unused print_prefixed_error method.
…age is corrected - Added #[ignore] attribute to test_only_one_input_file to skip it during execution. - Reason: Test likely fails due to an incorrect error message; this prevents false negatives while the message is being fixed in the tsort utility.
|
GNU testsuite comparison: |
CodSpeed Performance ReportMerging #9289 will not alter performanceComparing Summary
Footnotes
|
|
GNU testsuite comparison: |
|
is it expected that tsort.pl doesn't pass yet? thanks |
- Change FILE arg to accept zero or more inputs (appended), defaulting to "-" if none - Add validation to error on more than one input with "extra operand" message - Update test to expect new error format, matching GNU tsort behavior - Unignore test_only_one_input_file after error message correction
Split the TSORT_EXTRA_OPERAND_ERROR constant string into multiple lines to improve code formatting and adhere to line length guidelines.
Removed the tests_tsort.patch entry as it is no longer applied, possibly due to upstream integration or obsolescence, to keep the patch series current and relevant.
|
GNU testsuite comparison: |
…rapper Remove unnecessary `.into()` call when creating the extra operand error in uumain, resulting in cleaner, more concise error handling code. This change does not alter the program's functionality but improves code readability and reduces nesting.
|
GNU testsuite comparison: |
src/uu/tsort/src/tsort.rs
Outdated
| return Err(USimpleError::new( | ||
| 1, | ||
| format!( | ||
| "extra operand {}\nTry 'tsort --help' for more information.", |
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.
please use the translate!() macro
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.
fix
src/uu/tsort/src/tsort.rs
Outdated
| )); | ||
| } | ||
|
|
||
| let input = inputs.into_iter().next().expect("at least one input"); |
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.
same
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.
fix
Add localized strings for 'extra operand' and 'at least one input' errors in en-US and fr-FR locales. Update code to use translate! macro for consistent error reporting across languages, improving user experience for international users.
The translate! macro returns a String, but expect() requires a &str. Added .as_str() to convert the translated string for correct type usage and fix compilation error.
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
Summary
Align tsort with GNU behavior: silently accept -w, reorder successors to produce GNU’s deterministic output, and surface loop errors with the new util: error: … prefix via clap_localization::print_prefixed_error.
Adjust uniq, comm, tsort, and chroot tests to expect the prefixed error strings so they continue to pass with the updated localization handling.
Keep the test fixtures and assertions consistent with the updated message format across the suite.
Tests
cargo test test_tsort
cargo test test_uniq (including gnu_tests)
cargo test test_chroot --features chroot