I am big +1 on this and will personally use it. Thanks!

With a commands.push.check-dirty (or something similar) that control the default value/behavior of this flag.

Kyle message made me realize that I had not sent mine, so I'm doing that now.

The ability to use --no- variants is fairly new (a couple of year maybe ?), but I (and others) have been trying to put it to good use since then. Since is quite new I don't think they so many of of such flag yet, but a notable example of that approach is --merge that I use daily.

I agree that in this case one of the variants seems less useful as the other, but it does not seem useless either. For example it has obvious usage in script and alias. Having it can also help to build a more consistent experience around boolean flag. (and who knows what users would actually use)

Regarding check vs allow, I find no-check better than no-allow (that I feel a bit awkward). Regarding consistency with existing options, hg update has as notable --check flag, that check for a dirty working directory.

In D11680#179988, @marmoute wrote:

The ability to use --no- variants is fairly new (a couple of year maybe ?), but I (and others) have been trying to put it to good use since then. Since is quite new I don't think they so many of of such flag yet, but a notable example of that approach is --merge that I use daily.

As in hg update --no-merge? I don't see how that does anything, I might be missing something?

@command(
    b'update|up|checkout|co',
    [... (b'm', b'merge', None, _(b'merge uncommitted changes')), ...]
)
def update(ui, repo, node=None, **opts):
    ...
    merge = opts.get('merge')
    ...
    if check:
        updatecheck = b'abort'
    elif merge:  # Truthiness check only
        updatecheck = b'none'

The only way that might do something is if you have something in [defaults] that sets --merge to true, such that this overrides it.

I agree that in this case one of the variants seems less useful as the other, but it does not seem useless either. For example it has obvious usage in script and alias. Having it can also help to build a more consistent experience around boolean flag. (and who knows what users would actually use)
Regarding check vs allow, I find no-check better than no-allow (that I feel a bit awkward). Regarding consistency with existing options, hg update has as notable --check flag, that check for a dirty working directory.

I'm trying us to the most desirable end state, which I hope is that this gets added to tweakdefaults, at which point we'd have the following scenarios:

• tweakdefaults enabled: user runs hg push and it fails, telling the user to run hg push --allow-dirty; they do so. If they hit this frequently, they either disable this config knob or learn to type hg push --allow
• tweakdefaults not enabled [current state]: user runs hg push and it succeeds, uploads even though they have a dirty working directory. Never learns about the config option.
• script (which should be using HGPLAIN=1 to disable tweakdefaults): hg push behaves like before, hg push --no-allow-dirty works if they actually want that behavior.
• aliases: I imagine it'll be rare that people make an alias like safepush = push --no-allow-dirty, but this is still something that the user does very infrequently, while I expect the interactive typing to be more common

In this set of cases, the first one is the one I think we should be optimizing for. The other three are cases where writing it like --check-dirty makes sense, but I would argue that it doesn't matter that there's more cases, because the number of times it happens (for scripts and aliases) is much lower, and the interactive user in the second case would be much better served by enabling tweakdefaults (or at least this config option).

In my opinion, flag negations should be rare. We also have the problem that if we're actually creating a tri-state boolean, we currently do not have any mechanism in the flag parsing code to show those properly in the help text. This will show up as:

--allow-dirty                  allow pushing with a dirty working copy, overriding
                               commands.push.check-dirty=abort (EXPERIMENTAL)

with no [no-] prefix, because it's not recognized as a tribool.

In D11680#180023, @spectral wrote:

In D11680#179988, @marmoute wrote:

The ability to use --no- variants is fairly new (a couple of year maybe ?), but I (and others) have been trying to put it to good use since then. Since is quite new I don't think they so many of of such flag yet, but a notable example of that approach is --merge that I use daily.

As in hg update --no-merge? I don't see how that does anything, I might be missing something?

@command(
    b'update|up|checkout|co',
    [... (b'm', b'merge', None, _(b'merge uncommitted changes')), ...]
)
def update(ui, repo, node=None, **opts):
    ...
    merge = opts.get('merge')
    ...
    if check:
        updatecheck = b'abort'
    elif merge:  # Truthiness check only
        updatecheck = b'none'

The only way that might do something is if you have something in [defaults] that sets --merge to true, such that this overrides it.

You are right, I expected this to work but this wasn't implemented, the --merge properly override the config, but --no-merge did not override the other way around.

So I fixed it in D11851, and it now does.

I agree that in this case one of the variants seems less useful as the other, but it does not seem useless either. For example it has obvious usage in script and alias. Having it can also help to build a more consistent experience around boolean flag. (and who knows what users would actually use)
Regarding check vs allow, I find no-check better than no-allow (that I feel a bit awkward). Regarding consistency with existing options, hg update has as notable --check flag, that check for a dirty working directory.

I'm trying us to the most desirable end state, which I hope is that this gets added to tweakdefaults, at which point we'd have the following scenarios:

• tweakdefaults enabled: user runs hg push and it fails, telling the user to run hg push --allow-dirty; they do so. If they hit this frequently, they either disable this config knob or learn to type hg push --allow
• tweakdefaults not enabled [current state]: user runs hg push and it succeeds, uploads even though they have a dirty working directory. Never learns about the config option.
• script (which should be using HGPLAIN=1 to disable tweakdefaults): hg push behaves like before, hg push --no-allow-dirty works if they actually want that behavior.
• aliases: I imagine it'll be rare that people make an alias like safepush = push --no-allow-dirty, but this is still something that the user does very infrequently, while I expect the interactive typing to be more common

In this set of cases, the first one is the one I think we should be optimizing for. The other three are cases where writing it like --check-dirty makes sense, but I would argue that it doesn't matter that there's more cases, because the number of times it happens (for scripts and aliases) is much lower, and the interactive user in the second case would be much better served by enabling tweakdefaults (or at least this config option).
In my opinion, flag negations should be rare. We also have the problem that if we're actually creating a tri-state boolean, we currently do not have any mechanism in the flag parsing code to show those properly in the help text. This will show up as:

--allow-dirty                  allow pushing with a dirty working copy, overriding
                               commands.push.check-dirty=abort (EXPERIMENTAL)

with no [no-] prefix, because it's not recognized as a tribool.

I can see multiple questions here with an agreement level varying from one question to another. So let me try to split them apart for clarity.

Should there be a config for this behavior ?

In my opinion : Yes,
(and I think we have a consensus)

Should this be a tri-state option ? (True/None/False; that override the config value when ≠ None)

In my opinion : Yes, that seems more logical and make good use of --no-prefix

(I am unclear about what the consensus currently is)

Should Flag negation be rare ?

I disagree here. We should use clearest available verb for flags, using negation to get the meaning we need. I would draw a similarity with boolean naming in the code, where we don't have a preferred "default" state for the common case, but we phrase the boolean in a way that is easy to read.

In addition, having a mix of negation and non-negation as common case will help make the (negation) feature more discoverable.

What's your rationale for wanting them to be rare ?

Should the proposed behavior be part of tweakdefault ?

I don't think so, hg push is reasonably independent from the working copy and it is a common use-case to push with uncommitted changes (pushing the working copy parent or not). Making it disabled by default seems weird to me.

A related behavior/config is commands.commit.post-status that display information about dirty content after the commit (including about unknown files !). I feel like it could catch most¹ of the error case you are trying to catch here (or at least, it would in my case). With a much smoother impact on the common workflow.

[1] even more since it would catch the "forgot to add a file" case.

Should we have a "warning" version of this behavior ?

In my opinion : Yes, it brings most of the value with very few drawbacks.

Should we use --allow-dirty-wc or --no-check-dirty-wc.

I prefer the --check wording :

• it is more consistent with hg update --check,
• the --no-check version is clearer that a validation step have been explicitly dropped

Should Flag negation be rare ?

I disagree here. We should use clearest available verb for flags, using negation to get the meaning we need. I would draw a similarity with boolean naming in the code, where we don't have a preferred "default" state for the common case, but we phrase the boolean in a way that is easy to read.
In addition, having a mix of negation and non-negation as common case will help make the (negation) feature more discoverable.
What's your rationale for wanting them to be rare ?

The biggest reason is because there's currently, as far as I know, no support in Mercurial's flag parser code for tribools, so these won't be annotated with the [no-] prefix in the help text.

Should the proposed behavior be part of tweakdefault ?

I don't think so, hg push is reasonably independent from the working copy and it is a common use-case to push with uncommitted changes (pushing the working copy parent or not). Making it disabled by default seems weird to me.

I can't describe how many times the internal version of this check we have at Google has saved me from making a mistake. I would recommend that you check whether you truly do want to do this frequently or not :)

A related behavior/config is commands.commit.post-status that display information about dirty content after the commit (including about unknown files !). I feel like it could catch most¹ of the error case you are trying to catch here (or at least, it would in my case). With a much smoother impact on the common workflow.

I did not know about that and I just turned it on, because that sounds really nice. But this would not actually cover what we're trying to do, as it requires the user to remember to run commit or whatever, and that's what we're trying to check that they did :P

[1] even more since it would catch the "forgot to add a file" case.

Should we have a "warning" version of this behavior ?

In my opinion : Yes, it brings most of the value with very few drawbacks.

Should we use --allow-dirty-wc or --no-check-dirty-wc.

I prefer the --check wording :

• it is more consistent with hg update --check,
• the --no-check version is clearer that a validation step have been explicitly dropped

I guess this comes down to whether we expect this in tweakdefaults or not; regardless of whether it's actually going to be in tweakdefaults, we're going to enable it at Google for everyone. When combined with my preference for not having flag negations, we end up with the --allow wording, and that's what we currently have it named internally. I really think it *should* be added to tweakdefaults, but I'll admit that my own personal usage of Mercurial might not match what non-Google users of Mercurial do, and maybe this causes issues with some workflows there.

Like I said before, I also think it's going to be exceedingly rare, if this isn't part of tweakdefaults, for someone to *enable* on the commandline. No one is going to start typing hg push --check, they'll either forget to specify it, or they'll turn on the option somehow. So in my mind the flag is purely an "escape hatch" for "no, I really want to do this", and the word "allow" conveys that quite well.

In D11680#182205, @spectral wrote:

Should Flag negation be rare ?

I disagree here. We should use clearest available verb for flags, using negation to get the meaning we need. I would draw a similarity with boolean naming in the code, where we don't have a preferred "default" state for the common case, but we phrase the boolean in a way that is easy to read.
In addition, having a mix of negation and non-negation as common case will help make the (negation) feature more discoverable.
What's your rationale for wanting them to be rare ?

The biggest reason is because there's currently, as far as I know, no support in Mercurial's flag parser code for tribools, so these won't be annotated with the [no-] prefix in the help text.

I see, "None" and "True" value get treated the same in that regard. It seems a small technical limitation that is fairly easy to lift. Do you have other reasons to be against them ?

Should the proposed behavior be part of tweakdefault ?

I don't think so, hg push is reasonably independent from the working copy and it is a common use-case to push with uncommitted changes (pushing the working copy parent or not). Making it disabled by default seems weird to me.

I can't describe how many times the internal version of this check we have at Google has saved me from making a mistake. I would recommend that you check whether you truly do want to do this frequently or not :)

I am curious if the warning version of this would give you most of the benefit with none of the anoyance. Did you gave it a try ? (see my previous comment about "forgetting to add a files" as a bonus)