In D8376#125174, @pulkit wrote:

In D8376#125173, @marmoute wrote:

I would prefer you wait for planned change to land before you do further change to the file. The coming changes fixes bug and I rather not have to fix too many conflict while doing so.

I already pushed them, I will resolve conflicts on those couple of patches and update them on heptapod.

This one is trivial to move ahead of the other two patches so I'll queue it and fix any conflicts in flight.

Heads up that we ran into a fun bug that was probably caused by this patch. I think what happened was this:

In D8281#124246, @spectral wrote:

In D8281#124129, @durin42 wrote:

In D8281#124058, @marmoute wrote:

Since narrow is still experimental, I don't think we should try too hard for backward compatibility. We could introduce a new end-point for a new encoding and drop the old one in a couple of version.

+0, honestly. I won't require it, but I'd really rather we shaved this yak _now_ rather than when narrow has even more users.

I'm getting a bit frustrated with how much time I've spent on this, made worse by the fact that I agree with everything everyone's saying and so it's not like I'm frustrated at the review process, just how slow I've been at accomplishing this.
So, before I go down another rabbit hole, here's what I'm thinking:

• Server emits a new capability narrow-exp-1-escaped (in addition to the current narrow-exp-1, this is not replacing the existing capability)

The parent patch (D8189) was a bit controversial. We don't need that patch if we apply the following patch on top of this one (making this test sleep-free):

In D8243#124077, @marmoute wrote:

In D8243#123559, @martinvonz wrote:

This is pretty ugly and it doesn't seem that the next patch depends on it. You said you'll soon clean it up anyway, so I wonder if should just wait for the better solution instead. It doesn't seem like this fixes a serious bug so we have to rush it. Thoughts?

My initial motivation to rush the ugly way was "getting the behavior right to compare with the changeset centric one and being able to test performance improvement for the changeset centric one while having access to a specific repository". However, cancelling of all travel has cancelled the window to access that repo. I'll resubmit a cleaner versions soon.
since you did not commented on it, I assume the new behavior is fine by you, right?

In D8289#124060, @marmoute wrote:

In D8289#124009, @pulkit wrote:

I like the idea. IIRC, Ryan from FB hit similar issues in a sprint some years ago and came up with hg up --finish or something like that.
Maybe we should not let user clear the mergestate and suggest continue/<cmd-name> --continue if it's not result of update command. Thoughts?

Ho, that's a good idea. It looks like hg update --merge is the only command that do not have --continue support. So instead of adding a whole new flag and action to this exception, removing the exception seems like a better move. What do you think @martinvonz ?

In D8284#123873, @marmoute wrote:

In D8284#123872, @martinvonz wrote:

In D8284#123838, @marmoute wrote:

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?

In D8284#123838, @marmoute wrote:

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

I've updated the synopsis (also added another patch in the series for adding -r to the synopsis).

In D8282#123675, @marmoute wrote:

Can you drop your change to tests/test-push-race.t from this patch ?

In D8282#123665, @marmoute wrote:

That's a good point, doing it for #require too would be more consistent.
I would make a difference between test that do not make too much of an effort to have a title + early documentation and the one who actually make effort to have a formal format for their title and documentation (the one with =====\ntitle\n===== or title\n=====).
I would be fine with having the requires/testcases right after the title when it make senses. I think there are some instance where the variants are formally documented, and the #testcase block is part of that. It might make sense to keep them at that location.

In D8282#123663, @marmoute wrote:

In D8282#123661, @martinvonz wrote:

(like imports and includes in other languages)

Precisely, import and includes usually comes after the initial module licence, title and documentation. So I would like the testcase declaration to comes after the initial module title (and maybe doc).

In D8281#123659, @mharbison72 wrote:

In D8281#123625, @spectral wrote:

In D8281#123621, @mharbison72 wrote:

The Windows path changes seem like a good idea.
Would quoting paths with commas eliminate the need for custom escaping? I don't feel strongly about it, but custom escaping always feels weird to me. (I fact, a coworker did some homebrew escaping for CSV files a few days ago, but I forget how it ultimately ended up.)

Let me play with that a bit, I think it'll work and be detectable on the server since the first character can't currently be a double-quote, so there wouldn't really be any BC issues aside from the pconvert (which wouldn't be as important anymore, but still probably a good idea?)

I haven't played with narrow yet, so I'm not sure of the context. Are these user input paths that would end up being ignored/rejected if a Windows user used path\to\file when talking to a Unix server? Or are these stored in a tracked file? (Which I think could still cause problems.) I can't think of a good reason to stay inconsistent, and it is still experimental, so it still seems like a good idea while we still can fix it.

I very much agree with Matt