substitute_one_step: stop after first variable has been substituted

[cre4ture]

This change provides the posibility to only replace the next variable in a given string, returning the position after the variable name.
This is usefull if the substitution is to be integrated into an existing string parsing statemachine.
E.g. I'm integrating in into a "shell-words" like funktionality needed by the uutils/coreutils-env app.

To avoid that we need to do a fork of your code, we want to bring it into your main branch.
Please tell me what you think.

Kind regards

[de-vri-es]

Hey! Sounds interesting. But maybe we can make it return a tuple of 3 slices instead of a Option<(String, usize)>: the leading part of the string before the next variable, the substitute value, and the remainder of the unprocessed string.

[cre4ture]

You where right, the interface was a bit mixed up. I did refactorings and I think this fits now more to your idea.
What do you think?

[de-vri-es]

I think we still need to prevent returning a String, since it will re-allocate every time a change is made now.

But I also wonder, do you still need substitute_one(...) if we have a parse(..) -> Template function? Or can you achieve your desired end result already if parsing and performing substitution is split?

[de-vri-es]

For unit tests, we use assert2 :)

[de-vri-es]

Don't replace these. The check!() and assert!() macros from assert2 give far better error messages than assert_eq!().

[de-vri-es]

Same here, don't change this. assert2::assert!() is better.

[de-vri-es]

Suggested change

	assert_eq!(result.0, "subst");
	assert_eq!(result.1.slice_before_ends, 6);
	assert_eq!(result.1.slice_after_starts, 11);
	assert_eq!(result.1.subst_type, SubstitutionType::Variable);
	assert!(result.0 == "subst");
	assert!(result.1.slice_before_ends == 6);
	assert!(result.1.slice_after_starts == 11);
	assert!(result.1.subst_type == SubstitutionType::Variable);

[de-vri-es]

Suggested change

	let result = substitute_one_step(source, &variables).unwrap().unwrap();
	let_assert!(Ok(Some(result)) = substitute_one_step(source, &variables));

[de-vri-es]

Suggested change

	let result = substitute_one_step(source.get(result.1.slice_after_starts..).unwrap(), &variables)
	.unwrap()
	.unwrap();
	assert_eq!(result.0, "$");
	assert_eq!(result.1.slice_before_ends, 6);
	assert_eq!(result.1.slice_after_starts, 8);
	assert_eq!(result.1.subst_type, SubstitutionType::UnescapeOne);
	let_assert!(Ok(Some(result)) = substitute_one_step(source.get(result.1.slice_after_starts..).unwrap(), &variables));
	assert!(result.0 == "$");
	assert!(result.1.slice_before_ends == 6);
	assert!(result.1.slice_after_starts == 8);
	assert!(result.1.subst_type == SubstitutionType::UnescapeOne);

[de-vri-es]

Suggested change

	let result = substitute_one_step(source, &variables).unwrap();
	assert!(result.is_none());
	assert!(let Ok(None) = substitute_one_step(source, &variables));

[cre4ture]

But I also wonder, do you still need substitute_one(...) if we have a parse(..) -> Template function? Or can you achieve your desired end result already if parsing and performing substitution is split?

I would still need a funktion like parse_one_template_part as I only trigger the variable substitution when I detected that there is a $. The part of the input after that variable is again parsed with my own parser.

But we can go for the template based approach anyway if you like. Just let me know what you thing about #14

[de-vri-es]

I would still need a funktion like parse_one_template_part as I only trigger the variable substitution when I detected that there is a $. The part of the input after that variable is again parsed with my own parser.

I think I'm not entirely seeing your use case clearly. It feels somewhat odd to perform only partial variable substitution. Wouldn't it make more sense to run your own parser, and then run the substitution on the relevant parts that your parser indicates?

Or maybe the opposite makes sense: parse a full template first, and then run your parser on the non-variable parts of the template?

Do you have a link to how you're using this maybe? Or otherwise could you explain with some pseudo-code how this is supposed to integrate with your parser?

But we can go for the template based approach anyway if you like. Just let me know what you thing about #14

I will! Probably not today though, as I'm on holiday :p So I have only limited time for reviews. But the comments about the unit tests apply to both PRs. If you have the time you could already process those.

[cre4ture]

This pull request contains (temporarily) the modified subst such that it works. If you like you can have a look at it:
uutils/coreutils#5801

I think the main problems I encountered when I tried to use the unmodified subst where these:

• When running the subst befor or after my parser: Unescaping was done twice such that some tests where this is relevant where failing. E.g.\$NAME should not be substituted, but was because my parser needs to deal and remove the escape char for other cases anyway.
• When running subst only on the variables: I would need to implement the detection of the end of the variable name myself. Only this way I can forward the exact relevant part.

With the "one-step" way, I just need to stop at an unescaped $ and call subst_one_step which tells me about the end of the variable name and the substituted value.

In the beginning, when I didn't know the subst source code that well, for me the easiest way to achieve my goal was to provide access to this single/one-substitution step. Now, as I know your code pretty well, I detected that maybe the only function that I need from subst is the parse_variable. Making that function public woult probably be enought.

[de-vri-es]

Hmm, interesting. I still like the template approach, but I also like to unblock progress for uutils. I'll have a look at that PR.