chmln / sd

Intuitive find & replace CLI (sed alternative)
MIT License
5.72k stars 136 forks source link

Handle "-" in the beginning of patterns (fixes #88) #262

Closed twolodzko closed 10 months ago

twolodzko commented 10 months ago

The README makes an invalid statement that the arguments cannot start with "-". They can and it is natively handled by Clap. With the fix, the patterns can start with "-". Of course, patterns like "-h" would be shadowed by the flags, so would need "--", e.g. sd -- -h X replaces "-h" with "X".

CosmicHorrorDev commented 10 months ago

Personally I don't think that this is a good idea to change. Allowing for hyphens to start values (particularly positional arguments) introduces a lot of ambiguity on which arguments are flags and which are values. This ambiguity ends up significantly degraded the error messages that clap produces which is the most compelling part of using clap to me

twolodzko commented 10 months ago

@CosmicHorrorDev agree in general, but sd has just a few flags, so it does not seem to be a problem in this particular case.

CosmicHorrorDev commented 10 months ago

That adds a hazard for us adding new flags in the future though, since one version may interpret something like --interactive as a value while the next would interpret it as a flag. Same goes for anyone using an older version of sd while trying to follow documentation that includes more flags (or in our case someone may expect sd to have some flags from sed that it doesn't)

Considering that passing values that start with hyphens likely isn't very common I would prefer to keep our current behavior. The current error message when trying to pass a value that starts with a hyphen seems to be sufficient in my opinion

image

twolodzko commented 10 months ago

I understand your arguments, though, from the user's perspective, I'd care much more about a flawless experience with handling regular expressions than the flags that don't do much in the case of sd. Also, you seem to be bumping the major version to 1.0.0 so breaking changes did not sound like something that painful. But I won't argue as this only solves a minor inconvenience.

nc7s commented 10 months ago

It's generally recommended to do something like sd [OPTIONS] -- FIND REPLACE [FILES]. Adding a -- is much easier and simpler than to deal with all the complexities of accepting option-like arguments. From a user's perspective, you just stick to this notion, then forget about whether a specific tool needs escaping or not.

twolodzko commented 10 months ago

Ok, I think you convinced me. I just got the impression from README and the GitHub issues that this is not implemented because it is impossible or due to how Bash handles flags, etc which is not the case since it can be easily done. But it seems like canonical software (e.g. grep) does the same.