Skip to content

Rename uniq --ignore-case long flag name to --insensitive#7191

Closed
webbedspace wants to merge 2 commits intonushell:mainfrom
webbedspace:insensitive
Closed

Rename uniq --ignore-case long flag name to --insensitive#7191
webbedspace wants to merge 2 commits intonushell:mainfrom
webbedspace:insensitive

Conversation

@webbedspace
Copy link
Copy Markdown
Contributor

@webbedspace webbedspace commented Nov 22, 2022

Description

sort, sort-by, str contains and find all have a -i flag that controls case-sensitivity. However, unlike uniq, the expanded version is called --insensitive for all of these commands. This changes uniq to use the same name. While this is a compat break, since long names aren't especially popular (existing primarily for self-documentation purposes), I consider this on the shallow end of the compat-break scale.

uniq -i remains exactly the same.

User-Facing Changes

uniq --ignore-caseuniq --insensitive (compat break)

Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

  • cargo fmt --all -- --check to check standard code formatting (cargo fmt --all applies these changes)
  • cargo clippy --workspace --features=extra -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect to check that you're using the standard code style
  • cargo test --workspace --features=extra to check that all tests pass

After Submitting

If your PR had any user-facing changes, update the documentation after the PR is merged, if necessary. This will help us keep the docs up to date.

@fdncred fdncred added the notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes label Nov 22, 2022
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Nov 22, 2022

If mine was the only vote, I'd change every existence of --insensitive to --ignore-case just because it's more intuitive to me.

@webbedspace
Copy link
Copy Markdown
Contributor Author

I prefer insensitive myself because it doesn't have a hyphen in it, and it's easier to remember what -i does if its full name is "insensitive" rather than "ignore". To me, "ignore" just makes me say "ignore what?"

@kubouch
Copy link
Copy Markdown
Contributor

kubouch commented Nov 22, 2022

+1 for --ignore-case. --insensitive is a bit vague: Insensitive to what? We could also go all the way for --case-insensitive.

@webbedspace
Copy link
Copy Markdown
Contributor Author

Your honour, I think the word "sensitive" by itself in a purely programming context pretty much only means "case-sensitive", does it not? So the word "insensitive" has the same meaning. (Meanwhile, we've got at least four commands with --ignore-errors as their expansion of -i, which is another "ignore" flag.)

@rgwood
Copy link
Copy Markdown
Contributor

rgwood commented Nov 22, 2022

Some prior art:

GNU grep: --ignore-case
ripgrep (rg): --ignore-case
The Silver Searcher (ag): --ignore-case
fd: --ignore-case

@webbedspace
Copy link
Copy Markdown
Contributor Author

Some prior art:

GNU grep: --ignore-case ripgrep (rg): --ignore-case The Silver Searcher (ag): --ignore-case fd: --ignore-case

These are a little unfair, since the middle two are obliged to copy the API of the first one…

Although, a proper manpage search reveals that less, tree and wget also use --ignore-case. I'll recognise that as a standard.

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Nov 22, 2022

@webbedspace one thing that would be helpful is standardizing on --ignore-case throughout our commands. I mention this in case you're interested in putting together a PR.

fdncred pushed a commit that referenced this pull request Nov 22, 2022
# Description

Support for this breaking change was raised in #7191. This affects
`sort`, `sort-by`, `str contains` and `find`. `--ignore-case` is used by
a few POSIX programs such as `less` and `grep`, as well as a few other
popular utils like `tree` and `wget`. Since long names aren't especially
popular (existing primarily for self-documentation purposes), I consider
this on the shallow end of the compat-break scale.

Note that the `-i` short flag is not affected.
 
# User-Facing Changes

See above.

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace --features=extra -- -D warnings -D
clippy::unwrap_used -A clippy::needless_collect` to check that you're
using the standard code style
- `cargo test --workspace --features=extra` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants