cut: show error for multiple mode args (-b, -c, -f)

[wolimst]

Overview

uutils-cut runs without an error when there are multiple cutting mode arguments of one type, i.e. more than one --bytes, or --characters, or --fields, while GNU cut shows an error as described in GNU cut manual - "Use one, and only one of -b, -c or -f."

For example:

# uutils-cut
$ echo "12345" | cargo run cut -c1 -c5
5

# GNU cut
$ echo "12345" | cut -c1 -c5
cut: only one list may be specified
Try 'cut --help' for more information.

Cause

Duplicated arguments were overriding the previous ones, therefore only the last cutting mode argument (-b, -c, -f) was handled, as you can see on the example above.

Fix

Disable overriding of cutting mode arguments (-b, -c, -f) using ArgAction::APPEND. Count the number of the cutting mode arguments and show an error when there are more than one -b, -c, or -f.

fixes #5925

[wolimst]

I used existing error message which was already being used when different types of mode args were given

cut: invalid usage: expects no more than one of --fields (-f), --chars (-c) or --bytes (-b)

but it is not exactly equal to the GNU's error message. Should the error message equal to GNU's?

[tertsdiepraam]

Excellent! Could you annotate this a bit with some comments explaining the issue and how we work around it. For now, the error message is fine, we can change it later if we want to.

[tertsdiepraam]

Suggested change

	let mode_args_count = [
	matches.indices_of(options::BYTES),
	matches.indices_of(options::CHARACTERS),
	matches.indices_of(options::FIELDS),
	]
	.into_iter()
	.filter_map(|mode| mode.map(|indices| indices.len()))
	.sum();
	let mode_args_count = [
	matches.indices_of(options::BYTES),
	matches.indices_of(options::CHARACTERS),
	matches.indices_of(options::FIELDS),
	]
	.into_iter()
	.filter_map(|mode| mode.map(|indices| indices.len()))
	.sum();

I tried to find a way of expressing this without the nested map. This is what I landed on. Still the still in functionality though. I think this is a pretty good way of doing it!

[wolimst]

I was also wondering for more clean way to do it to remove nested map. It looks like the suggested change is identical to the original one, could you edit it to what you landed on?

[cakebaker]

I don't know what @tertsdiepraam had in mind, here's an idea of how to get rid of the nested map:

let mode_args_count = [
    matches.indices_of(options::BYTES),
    matches.indices_of(options::CHARACTERS),
    matches.indices_of(options::FIELDS),
]
.into_iter()
.map(|indices| indices.unwrap_or_default().count())
.sum();

[wolimst]

Thanks @cakebaker! It looks like a nice way to transform Option::None to an empty iterator! Also, using count() instead of len() seems more correct way to do it, since it looks like the intended use case of len() is to get the remaining length of the iterator.
Just updated the code!

[tertsdiepraam]

I must've made a mistake while copying code from my editor. I'll take another look in a bit. Sorry for the confusion 😄

[tertsdiepraam]

let mode_args_count = [
    matches.indices_of(options::BYTES),
    matches.indices_of(options::CHARACTERS),
    matches.indices_of(options::FIELDS),
]
.into_iter()
.flatten()
.map(|indices| indices.len()))
.sum();

This is what I had in mind I think.

[wolimst]

Thanks, added some comments about the issue and the fix.

[cakebaker]

Thanks for your PR :)