>> +`:<dst>` is optional. > > It may be technically true, but I am not sure if it is a good idea > to say it here. Without "when missing, <dst> is inferred with these > rules", saying just "is optional" naturally invites a puzzlement: > when do we need to supply it and for what? > > As you are going to say what happens when you omit it very soon, and > you already have said with [:<dst>] that it is optional, perhaps you > can scratch this sentence. The reason I mentioned this early on is that when I read this for the first time it was quite confusing. Since `main` (or another branch name on its own) was the only push refspec I had experience using, it was disorienting to be told that the format was `<src>:<dst>`. It made me wonder if maybe `main` was not a refspec after all and I'd misunderstood the SYNOPSIS. I know that I'm not alone in this: it's very common for Git users (even very experienced ones!) to learn the form `git push origin main`, but to otherwise not be familiar at all with the concept of a "refspec". So I'd like to do something here to help those folks connect their existing knowledge to the broader concept of a "refspec". That said `:<dst> is optional` may not be necessary: I think the examples here accomplish more or less the same thing. >> +The format for a refspec is [+]<src>[:<dst>], for example `main`, > > +`main:other`, or `HEAD^:refs/heads/main`. >> +`+` is optional and does the same thing as `--force`. > > Ditto; this one is less bad than the :<dst> thing, because at least > it tells us what it means. But we are going to talk about when an > update is not allowed (we haven't even hinted that some updates may > not be allowed yet) much later, "the same as `--force`" is probably > a bit premature at this point in the documentation. The reason I mentioned this so early is similar -- It's very common for experienced users to be familiar with the `--force` option, and so this is a quick way to explain to them what the `+` does. As an aside, I noticed while reading this that it looks like the description of `--force` right now is not quite accurate, since the rule about pushing to tags is not "it must be an ancestor". >--force:: > Usually, the command refuses to update a remote ref that is > not an ancestor of the local ref used to overwrite it. I'm going to think about whether there's a way to format the "rules for updates" so that they can be referred to from the `--force` section. Or maybe just move them to the `--force` option, and refer people there if they want to understand exactly when `--force` is required. It's certainly much more common for users to use `--force` to force a push than to use `+`, from that perspective it would make more sense to document the rules next to `--force` than next to the description of `+`. >> +You can write a refspec using the fully expanded form (for >> +example `main:refs/heads/main`) which specifies the exact source > > This example is not fully expanded. refs/heads/main:refs/heads/main > would be, though. Makes sense, will change. > I am not sure it is easier to read with numbered list. It is not > like these rules are applied in this order, or anything like that, > right? I had a similar concern. I'm still trying to figure out how to manage unordered lists, since lists with a `*` are formatted by AsciiDoc in a weird way in the terminal: there's a tab character after the • character which I don't understand the reason for. > A tangent. > Is this a refspec you can write in .git/config, e.g. > > [remote "origin"] > push = tag v1.0" > > If not, it might be easier to explain if we tweaked the command line > synopsis to say that the command takes, after the destination > repository, zero or more refspec or "tag <tag>". I dunno. I checked and it's not a valid refspec: $ git push fatal: invalid refspec 'tag v1.0' This list seems like a more natural place for that information than the synopsis though: the synopsis is already quite hard to read and we can't use it as the only place to communicate that information. I can add a note to say that 'tag v1.0' is technically not a refspec. Git already has enough weird exceptions like that that I don't think it'll be too jarring. > Perhaps move it down together with the "push void to remove" > at the top of the list? I'll also move it down. > Good. Somehow we have added the description of these to "git fetch" > side, without updating "git push" side of the documentation. Yes, I copied this part from the `git fetch` documention after checking that they worked with `git push` as well. >> + Deletions are always accepted without a leading `+` in the >> + refspec (or `--force`), except when forbidden by configuration or hooks. > > This can be read in two ways, making two opposing answers to this > question possible: when forbidden, can you make a deletion accepted > by giving a `+` in front? > "except when forbidden, deletions are accepted with or without `+`" > might be less confusion-prone, but I dunno. I just wanted to make > sure that it is clear that forcing or prepending `+` would not > change anything when forbidden by configuration or hooks on the > remote end. > > But because you haven't mentioned that not all updates are allowed, > this might be a bit out of place in this list. How about limiting > this bullet point to only say that this is the syntax to use to > delete a ref from the remote, and move the "deletions do not have to > be forced and operations forbidden at the remote cannot be forced > anyway" down, near the "Not all updates are allowed" below ... Will do, that's much cleaner. >> + See `receive.denyDeletes` in linkgit:git-config[1] and `pre-receive` and >> + `update` in linkgit:githooks[5]. > > ... together with this? This makes sense. - Julia
