RFC: A new approach to OsStr support #11
Conversation
"The feature `bench_black_box` has been stable since 1.66.0 and no longer requires an attribute to enable"
This is definitely a nitpick and mostly my own opinion. "option requires a value: ..." makes it sound like the option requires itself "option does not require a value: ..." makes it sound like the value is optional Since the text will always be something the programmer has agreed is an acceptable option. It should be fine to just embed it in the middle of the message.
This emulates the behaviour of python's argparse when you specify options that look like negative numbers in your parser (e.g. -1 or -.5 or -123.456 which are not all short options but argparse is versatile). This is useful if you take a optionally negative number as a positional argument. The default is to keep the normal behaviour with the alternative optionally enabled by calling Options::allow_number_short_options(..., false)
This is an idea stolen from rust-lang/rust#120048. It's useful for the next commit.
Instead of letting you choose what type you want to get back from the API, just make it a `&str`. This means that options can only ever be something which can be turned into valid UTF-8. This sounds restrictive but for users of `&str as Argument` it makes no difference and for the few users of `&[u8] as Argument` it simplifies a lot of things. It also makes `&[u8]` usable on windows via `OsStr::as_encoded_bytes` and makes `&OsStr as Argument` a possibility.
Implement `&OsStr as Argument` on top of `&[u8] as Argument`. The unsafe code to convert back from the sliced `&[u8]` keeps to the safety invariants defined for the `OsStr::from_encoded_bytes_unchecked` function.
|
Interesting! I have thought about changing the API before, mostly thinking about refactoring the code to mimic what Also, currently, the I haven't looked at your code yet, since I'm currently quite busy with university, but I'll try to get to it soon. Thanks for your contribution!! |
This is an interesting idea. I will try to look into incorporating it as an alternative branch at some point in the near future.
Yes I noticed this, but after working with the code a bit I realised that neither order made much sense. But your suggestion above would make this easier.
No worries, take your time. I will add comments if I make any changes/updates but I do not expect you to review anything. I really published this more as a discussion piece at this point in time. |
I would like to preface this by saying that I am happy to just create a fork of getargs with my ideas and leave this project be.
I have prepared a set of patches of progressive improvements which culminate in the addition of
&OsStr as Argumentsupport.I think more tests would be useful to have etc. But this was mainly a proof of concept for myself.
Additionally the patch set adds the feature of not negative numbers as options taken from argparse in python. This is optional.
Really, I am mostly interested in comments and feedback from others who have worked on this project as I think they may be best suited to evaluate the commits.
I am curious as to what I could have done better and if this could ever be merged.
Edit: It's also worth mentioning that this change does introduce some minor (10%-50%) performance regressions. For me this is acceptable given the performance is still above and beyond anything which I would ever be concerned by. But I would be curious to know if this can be improved on.