Page MenuHomePhabricator

fix: disallow `hg fix --all --working-dir`
ClosedPublic

Authored by martinvonz on Mar 13 2020, 3:33 PM.

Details

Summary

--all implies --working-dir, so it's probably a mistake if the
user uses both.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Mar 13 2020, 3:33 PM
mharbison72 accepted this revision.Mar 13 2020, 6:08 PM

If --working-dir and --all are redundant, I don't see anyharm in allowing both to be passed.

If --working-dir and --all are redundant, I don't see anyharm in allowing both to be passed.

The idea was to inform users that they're doing something that's a little weird, in case they were hoping for it to do something else. I don't care much and I'm fine with dropping this patch if that's the consensus.

If --working-dir and --all are redundant, I don't see anyharm in allowing both to be passed.

The idea was to inform users that they're doing something that's a little weird, in case they were hoping for it to do something else. I don't care much and I'm fine with dropping this patch if that's the consensus.

If the intend is to inform, maybe we could issue a warning?

If --working-dir and --all are redundant, I don't see anyharm in allowing both to be passed.

The idea was to inform users that they're doing something that's a little weird, in case they were hoping for it to do something else. I don't care much and I'm fine with dropping this patch if that's the consensus.

If the intend is to inform, maybe we could issue a warning?

Makes sense. I suppose we should do the same for --all and --rev REV in that case. We currently consider that an error. I don't care much about this issue, so I'll just move this patch to the side for now. That way the rest of the stack (which I care about) can be queued independently.

If --working-dir and --all are redundant, I don't see anyharm in allowing both to be passed.

The idea was to inform users that they're doing something that's a little weird, in case they were hoping for it to do something else. I don't care much and I'm fine with dropping this patch if that's the consensus.

If the intend is to inform, maybe we could issue a warning?

Makes sense. I suppose we should do the same for --all and --rev REV in that case. We currently consider that an error. I don't care much about this issue, so I'll just move this patch to the side for now. That way the rest of the stack (which I care about) can be queued independently.

I can't think of any existing precedent to follow, and it would probably be fine to error out or silently ignore it. But warning here feels weird- what's the user supposed to do after the command has run? There's already enough grief with the did you mean bookmark? warning, and that's something that the user could react to.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.