Skip to content
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

ls: silently ignore -T option #3718

Merged
merged 19 commits into from
Jul 26, 2022
Merged

ls: silently ignore -T option #3718

merged 19 commits into from
Jul 26, 2022

Conversation

Stonks3141
Copy link
Contributor

@Stonks3141 Stonks3141 commented Jul 14, 2022

The option can't be implemented without a significant rewrite according to #3624, and right now passing the option causes a failure.

  • fixed error when -T option is passed
  • write tests for -T option

@sylvestre
Copy link
Sponsor Contributor

i don't think we can mix clap 3.1 & 3.2
see
#3634 (comment)

ls: remove tests for tabsize arg validation
@Stonks3141
Copy link
Contributor Author

Ok, I reverted the changes and removed the relevant tests.

@sylvestre
Copy link
Sponsor Contributor

many CI jobs are failing, could you please have a look?
thanks

@Stonks3141
Copy link
Contributor Author

They all say

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.64.0-nightly (23e21bdd2 2022-07-15) running on x86_64-unknown-linux-gnu

note: compiler flags: --crate-type lib -C embed-bitcode=no -C debuginfo=2 -C incremental -Z binary-dep-depinfo -Z save-analysis

I don't think I changed anything that would break CI, I just added a Clap arg and a test function. I can replicate the error locally, but the error is in uu_od. It also occurs if I clone the upstream repo and run cargo +nightly udeps (udeps needs nightly).

@Stonks3141
Copy link
Contributor Author

Most of the CI jobs magically fixed themselves, and I fixed the style/spelling issues. Should be ready to merge now, unless you have any other issues?

tests/by-util/test_ls.rs Outdated Show resolved Hide resolved
@sylvestre sylvestre merged commit 4e72e28 into uutils:main Jul 26, 2022
pimzero pushed a commit to pimzero/coreutils that referenced this pull request Aug 1, 2022
* ls: silently ignore `-T` option
pimzero pushed a commit to pimzero/coreutils that referenced this pull request Aug 1, 2022
* ls: silently ignore `-T` option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants