Implement VariableMap for Vec<AsRef<str>>#31
Conversation
|
Hey! Thanks for the PR. The use case makes sense, but I'm not 100% sure if having a On the other hand, I can't really think of another interpretation for using a vector as a map, so maybe it's better to implement it directly on |
src/map.rs
Outdated
| if key == "0" { | ||
| let mut s = String::new(); | ||
| if let Some((first, rest)) = self.split_first() { | ||
| s.push_str(first.as_ref()); | ||
| for v in rest { | ||
| s.push(' '); | ||
| s.push_str(v.as_ref()) | ||
| } | ||
| } | ||
| Some(Cow::from(s)) |
There was a problem hiding this comment.
Hmm, this case seems a little odd: You seem to want "$0" to be used sort-of like "$@" is a shell?
This is quite surprising behavior to me though. Normally, "$0" would be the name of the program, not the concatenation of all arguments.
There was a problem hiding this comment.
ah, well spotted. I did have key == "*" there at some point. My use case doesn't have $0, but I wanted the numbering to start at 1, so I changed to this.
It probably makes sense to treat the Vec like a regular args vec (just returning the index, starting at 0), and using another key.
There was a problem hiding this comment.
I remember why I went with 0: * is rejected by the parser as invalid variable name.
There was a problem hiding this comment.
hmok, if making it possible to implement more shell-like behavior ($* returns all numbered args concatenated), I could add a special case for a name that's just `*, to the parser. Let me know!
Hm, doesn't |
So only implementing this for |
de944a7 to
e68c25d
Compare
|
I've updated the parser and made this more shell-like. |
|
Hey! Sorry for the delay. It's RustWeek in the Netherlands :) Now, the PR does a few different things:
I think we can merge 2. But I have some reservations on 1 and 3. I think single symbols as variable names are cryptic. I don't particularly like the choice of shells to use them a lot. And regarding 3, it feels to specific to one use case to put it in the I could see one way how this use case might be supported:
|
I'm there, too! :) Maybe we bump into each other, are you going to the 10 years of Rust celebration?
I did a custom VariableMap (basically the version in this PR but over a newtype holding a I'd appreciate support for those special single letter variable names, to make this behave more like expected (by users used to shell). If you'll have it, I'll create a new PR with just e68c25d (the commit allowing |
|
As also discussed at RustWeek: let's go ahead and support special characters as variable names: only single character special symbols, since that is what shells also do :) |
|
Since this library is only attempting to be shell-like, it could be neat (famous last words...) to be able to parse some simple ranges, eg |
In principle I'm okay with being more Rust-like as long as it's not too un-shell like. But I struggle to see what a range would mean here. I guess you would expect the variables to be joined by a single space character? My first feeling is that the choice of joining character is not that obvious, but if we allow specifying it we're getting to the realm of full-fledged template engines. And that would overshoot the goal of the library. |
I imagine that a range-substition would mean the same as |
This implements
VariableMapfor&[T]slices containing strings (T: AsRef<str>).In addition to this, the parser is updated to allow a single
*as variable name. This is used in above'sVariableMapimplementation to make$*and${*}substitute to the concatenation of all variables but `$0, like a shell would do.My use-case for this is too take an argument list like argv, and then use subst to replace those numbered arguments within a string.