djui / alias-tips

An oh-my-zsh plugin to help remembering those aliases you defined once
786 stars 50 forks source link

Find alias even when flag order is differently defined #18

Open djui opened 8 years ago

djui commented 8 years ago

Currently the following does not work:

# alias l=ls '-l -h -a'
# alias l=ls -lha'
$ ls -l -a -h foo
$ ls -lah foo

should be suggestion: l foo.

The algorithm should change to a heuristic that in most cases finds the alias. What makes this non-trivial is at least:

  1. -- must be respected as flag and argument terminator
  2. -foo is indistinguishable from -f -o -o for some commands but is actually --foo for other commands
  3. -f bar, --foo bar, and --foo=bar are not always identical depending on if -f|--foo is and argument flag or bool flag.

Acceptance criteria and constraints:

djui commented 8 years ago

@pradyunsg You might be interesting in watching this one. I'd also be glad for any suggestions and comments.

pradyunsg commented 8 years ago

Indeed I am interested.

I haven't thought about this yet. I'll let you know what I think when I do.

pradyunsg commented 8 years ago

As much as I would like this, it is inherently difficult to provide proper support for this...

-- must be respected as flag and argument terminator

I don't see how this is a non-trivial matter... It should be pretty simple to filter out the flags (loop with conditional break), till the -- or end-of-input only. Am I missing something?

-foo is indistinguishable from -f -o -o for some commands but is actually --foo for other commands

IMO the latter is not the most-common pattern CLI design. So, it should be fine to ignore it... Plus, I don't expect the first iteration to have support for multi-character options. In any case, it is possible to simply ignore that specific alias if it causes issues.

How about maintaining a list of commands that have this behaviour? No. Too much effort, too little reward.

-f bar, --foo bar, and --foo=bar are not always identical depending on if -f|--foo is and argument flag or bool flag.

This seems to be the blocker to me. It is very difficult to know if it's a flag or argument option... And assuming either would be in-correct... AFAICS, the only ambiguity exists with -f bar. Is it -f and bar or -f=bar? All other cases are not going to be handled by alias-tips (right?).

alias-tips can't know that -f is --foo for every tool. For popular ones, maybe add some configurable supporting infrastructure? It'll be complex and I don't think the effort to reward ratio is worth it.

It's tolerable if it introduces a neglect able amount of false-positives

Not for me. I do not want to be checking that what is being suggested to me by alias-tips, is actually what I just typed. It feels contrary to what it's supposed to do. I'd rather miss a few edge-cases than have a false positive.

It's not tolerable for run-time to go up significantly, as one would have to check for all permutations over all aliases defined.

:+1:

No regression should be introduced.

:+1: Obviously.


What do you think?

djui commented 8 years ago

Just a clarification (will respond to the rest later): When I write non-trivial I don't mean hard to do, but rather adding special cases to the code which will make it harder to read and more brittle.

djui commented 8 years ago

How about maintaining a list of commands that have this behaviour?

Basically all tools written in Go(lang) that use the standard flag parser have this behaviour. (I dislike it a lot).

Overall you made very good points and I believe our intersection in concerns is large. We could start with minimal support, starting with obvious, easy, and always (i.e. no best guesses, no assumptions, etc.) correct cases and then add support as we feel like and think the trade-offs are worth it.

pradyunsg commented 8 years ago

Basically all tools written in Go(lang) that use the standard flag parser have this behaviour. (I dislike it a lot).

Ah, Go. I lost enthusiasm for it ever since Rust came out... But that's another topic.

I don't like that behaviour at all, especially because both option styles (- and --) are handled the same way... :confused: Feels unnecessary...

Overall you made very good points

Thanks! :smile:

We could start with minimal support, starting with obvious, easy, and always (i.e. no best guesses, no assumptions, etc.) correct cases

Sounds good to me! :+1:

So, what is that minimal "safe" subset? I would like to ignore any argument options (how many arguments? Till next option? only one? can't know), for now. Flag options should be detect able.

In my head, this would be the processing:

djui commented 8 years ago

So I tried to come up with some test cases that define a minimal support set for this feature and declared and skipped other tests that are quite common real-life cases but are either hard or impossible to match. Please have a look at https://github.com/djui/alias-tips/pull/23 which only contains the test cases the feature should be spec'd against.

While doing so I encountered something we haven't discussed yet:

Even if we are not sure if a flag is argument flag (with argument provided) or boolean flag, if the alias mapping defines the flag and argument we should obviously match this. E.g.: alias a='foo -b bar -a baz'; foo -b bar -a baz qux => a qux. It looks trivial as even the current prefix-match strategy would find this, but if we would switch entirely to "canonical reordered-flags"-matching it looks impossible to distinguish (After the reordering). There is a fair chance of regressing if one would choose only "canonical reordered-flags"-matching imho. I haven't thought it through yet, but I could believe even with the new feature one would need to apply both strategies in parallel or sequential with prefix matching as higher priority. Counterexample: E.g.: alias a='foo -b bar'; foo -b bar -a baz qui vs alias a='foo -a baz'; foo -b bar -a baz qux => we wouldn't try this at all as we can't be certain if not considering prefix-matching.

So it might be that we have to consider "flag-reordering" not as replacement but as sequential and additional step after prefix matching: Either prefix matching did find a match, then consider flag-reordering the rest and try again to find something even better, or prefix matching did not find a match, then again consider flag-reordering but this time everything and try again to find something. If we would not do this there might be regressions, and in fact I know there will be just by looking at my own aliases (E.g. gsps='git show --pretty short --show-signature', used as git show --pretty short --show-signature foo would not be gsps foo).