Support allow_hyphen_values in native completions

[mart-mihkel]

Resolves #5594

[epage]

@shannmu would be interested in you looking over this as well, both from your experience with this code and for where you were planning on taking this code.

[shannmu]

Sorry for the late reply, I will look over the code.

[shannmu]

Thank you very much for your attention to this issue!

clap/clap_complete/src/dynamic/complete.rs

Lines 58 to 131 in e318821

	if is_escaped {
	(state, pos_index) = parse_positional(current_cmd, pos_index, is_escaped, state);
	} else if arg.is_escape() {
	is_escaped = true;
	state = ParseState::ValueDone;
	} else if let Some((flag, value)) = arg.to_long() {
	if let Ok(flag) = flag {
	let opt = current_cmd.get_arguments().find(|a| {
	let longs = a.get_long_and_visible_aliases();
	let is_find = longs.map(|v| {
	let mut iter = v.into_iter();
	let s = iter.find(|s| *s == flag);
	s.is_some()
	});
	is_find.unwrap_or(false)
	});
	state = match opt.map(|o| o.get_action()) {
	Some(clap::ArgAction::Set) | Some(clap::ArgAction::Append) => {
	if value.is_some() {
	ParseState::ValueDone
	} else {
	ParseState::Opt((opt.unwrap(), 1))
	}
	}
	Some(clap::ArgAction::SetTrue) | Some(clap::ArgAction::SetFalse) => {
	ParseState::ValueDone
	}
	Some(clap::ArgAction::Count) => ParseState::ValueDone,
	Some(clap::ArgAction::Version) => ParseState::ValueDone,
	Some(clap::ArgAction::Help)
	| Some(clap::ArgAction::HelpLong)
	| Some(clap::ArgAction::HelpShort) => ParseState::ValueDone,
	Some(_) => ParseState::ValueDone,
	None => ParseState::ValueDone,
	};
	} else {
	state = ParseState::ValueDone;
	}
	} else if let Some(short) = arg.to_short() {
	let (_, takes_value_opt, mut short) = parse_shortflags(current_cmd, short);
	match takes_value_opt {
	Some(opt) => {
	state = match short.next_value_os() {
	Some(_) => ParseState::ValueDone,
	None => ParseState::Opt((opt, 1)),
	};
	}
	None => {
	state = ParseState::ValueDone;
	}
	}
	} else {
	match state {
	ParseState::ValueDone | ParseState::Pos(..) => {
	(state, pos_index) =
	parse_positional(current_cmd, pos_index, is_escaped, state);
	}
	ParseState::Opt((opt, count)) => match opt.get_num_args() {
	Some(range) => {
	let max = range.max_values();
	if count < max {
	state = ParseState::Opt((opt, count + 1));
	} else {
	state = ParseState::ValueDone;
	}
	}
	None => {
	state = ParseState::ValueDone;
	}
	},
	}
	}
	}

This code is primarily used to match patterns in the arg string itself, such as --, --flag, -f, and value. These different patterns go into different branches. Adding an if allow_hyphen branch in there, I believe, would disrupt the logic of this code.

I think we could try matching the state in other branches, similar to how it's done in the else branch.
Here’s an example:

else if let Some((flag, value)) = arg.to_long() {
    if let ParseState::Opt(opt, count) = state {
        if opt.is_allow_hyphen_values_set() {
            // update state
        }
    }
}

Of course, the logic for allow_hyphen_values should be similar across these branches, so perhaps the code can be reused.

[shannmu]

Another case we need to test for and handle is when we are in the ValueDone state, if the long or short is unknown and the next positional allows hyphen values, then we will take the unknown argument and treat it as a value.

As mentioned above, we need to add some logic in each branch. If the result of current_cmd.get_arguments().find() is None and the next positional argument allows hyphen values, we need to update pos_index.
Additionally, we also need to make some adjustments in complete_arg to support the complete positional arguments that allow hyphen values.

No need to worry about the code getting bloated; if it does, that just means it’s time for a refactor.

[epage]

I've been trying to do some clean up to make it easier for people to be contributing to this code. Things should be calmed down for resolving conflicts.

[mart-mihkel]

I'm a bit unsure what I should change in the arg.is_escape case and in complete_arg

[shannmu]

current_cmd.get_arguments().find() is None means the current raw_arg looks like an option but it's not. If the current positional argument allows hyphen values, we could treat it as a positional argument, so we need to update pos_index.

Additionally, we also need to make some adjustments in complete_arg to support the complete positional arguments that allow hyphen values.

I seem to have misunderstood; updating pos_index is enough. There are no restrictions on completing positional arguments in complete_arg.

[epage]

Could you clean up your commits so they are presented for easy understanding of what you are trying to accomplish and without any merge commits?

[epage]

Thanks!