| Message ID | d48b4719c275ef06da014b6d22983db9ae484db2.1709590037.git.code@khaugsbakk.name (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | advise about ref syntax rules | expand |
Kristoffer Haugsbakk <code@khaugsbakk.name> writes: > 1. Use “shown” instead of “advice shown” > • “advice” is implied and a bit repetitive > 2. Use “when” instead of “if” > 3. Lead with “Shown when” and end the entry with the effect it has, > where applicable > 4. Use “the user” instead of “a user” or “you” > 5. detachedHead: connect clause with a semicolon to make the sentence > flow better in this new context > 6. implicitIdentity: rewrite description in order to lead with *when* > the advice is shown (see point (3)) > 7. Prefer the present tense (with the exception of pushNonFFMatching) > 8. Use a colon to connect the last clause instead of a comma > 9. waitingForEditor: give example of relevance in this new context > 10. pushUpdateRejected: exception to the above principles I'll let others comment on these as general principles. I do not immediately see anything objectionable, but I may change my mind after reading the updated text in the patch. > Suggested-by: Junio C Hamano <gitster@pobox.com> I am getting too much credit for this; I merely suggested to use "when" instead of "if" in the one you are newly adding. > detachedHead:: > - Advice shown when you used > + Shown when the user uses > linkgit:git-switch[1] or linkgit:git-checkout[1] > - to move to the detached HEAD state, to instruct how to > + to move to the detached HEAD state; instruct how to > create a local branch after the fact. I agree "Advice shown when" -> "Shown when" is a good change for brevity, but I do not think the other change is an improvement. This advice message is shown when the user does X, in order to instruct the user how to do Y after that. And "to instruct" is a common way to say the same thing as "in order to instruct". > implicitIdentity:: > - Advice on how to set your identity configuration when > - your information is guessed from the system username and > - domain name. > + Shown when the user's information is guessed from the > + system username and domain name: tell the user how to > + set their identity configuration. Should that be a colon? Stopping a half-sentence and connecting to another half-sentence is usually done with a semicolon (like you did in the new version of detachedHEAD above). Shown when ... and domain name, to tell the user how to set their identity configuration. perhaps? There may be other similar entries whose updated text uses colon followed by an imperative sentence, but I didn't look very carefully. > statusUoption:: > - Advise to consider using the `-u` option to linkgit:git-status[1] > - when the command takes more than 2 seconds to enumerate untracked > - files. > + Shown when linkgit:git-status[1] takes more than 2 > + seconds to enumerate untracked files: consider using the > + `-u` option. Earlier ones after a colon (or semicolon in detachedHEAD case), you gave an order to the advice message (e.g. "hey detachedHead advice, tell the user how to create a local branch"), but this one is giving an order to the end user, which feels inconsistent. I do not have a strong objection against giving an order to the advice message, as long as it is done consistently. If we did so, the part after the colon would start with "instruct the user ..." or "tell the user ..." and the like, and the gist of what this one would say would be "shown when it is taking too long: suggest the user to consider `-u`". FWIW, my earlier "in order to" took an approach that is different from either of the two "giving an order" approaches. I was trying to make the description explain what the message tries to do and/or why the message is given (e.g., "shown if it takes too long in order to suggest users to consider the -u option"). Thanks.
On Tue, Mar 5, 2024, at 00:52, Junio C Hamano wrote: >> detachedHead:: >> - Advice shown when you used >> + Shown when the user uses >> linkgit:git-switch[1] or linkgit:git-checkout[1] >> - to move to the detached HEAD state, to instruct how to >> + to move to the detached HEAD state; instruct how to >> create a local branch after the fact. > > I agree "Advice shown when" -> "Shown when" is a good change for > brevity, but I do not think the other change is an improvement. > > This advice message is shown when the user does X, in order to > instruct the user how to do Y after that. And "to instruct" is a > common way to say the same thing as "in order to instruct". Well argued. I’ll go back to the comma. >> implicitIdentity:: >> - Advice on how to set your identity configuration when >> - your information is guessed from the system username and >> - domain name. >> + Shown when the user's information is guessed from the >> + system username and domain name: tell the user how to >> + set their identity configuration. > > Should that be a colon? Stopping a half-sentence and connecting to > another half-sentence is usually done with a semicolon (like you did > in the new version of detachedHEAD above). > > Shown when ... and domain name, to tell the user how to set > their identity configuration. > > perhaps? There may be other similar entries whose updated text uses > colon followed by an imperative sentence, but I didn't look very > carefully. I’ll spoil it for you: there are a lot of colons. ;) Good point. I’ll go over it again and probably use more semicolons instead. >> statusUoption:: >> - Advise to consider using the `-u` option to linkgit:git-status[1] >> - when the command takes more than 2 seconds to enumerate untracked >> - files. >> + Shown when linkgit:git-status[1] takes more than 2 >> + seconds to enumerate untracked files: consider using the >> + `-u` option. > > Earlier ones after a colon (or semicolon in detachedHEAD case), you > gave an order to the advice message (e.g. "hey detachedHead advice, > tell the user how to create a local branch"), but this one is giving > an order to the end user, which feels inconsistent. > > I do not have a strong objection against giving an order to the > advice message, as long as it is done consistently. If we did so, > the part after the colon would start with "instruct the user ..." or > "tell the user ..." and the like, and the gist of what this one > would say would be "shown when it is taking too long: suggest the > user to consider `-u`". Yeah, I paused for a minute when writing that. I’ll change to “tell” or something similar. > FWIW, my earlier "in order to" took an approach that is different > from either of the two "giving an order" approaches. I was trying > to make the description explain what the message tries to do and/or > why the message is given (e.g., "shown if it takes too long in order > to suggest users to consider the -u option"). > > Thanks.
diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt index c7ea70f2e2e..cfca87a6aa2 100644 --- a/Documentation/config/advice.txt +++ b/Documentation/config/advice.txt @@ -6,23 +6,23 @@ advice.*:: + -- addEmbeddedRepo:: - Advice on what to do when you've accidentally added one + Shown when the user accidentally adds one git repo inside of another. addEmptyPathspec:: - Advice shown if a user runs the add command without providing + Shown when the user runs the add command without providing the pathspec parameter. addIgnoredFile:: - Advice shown if a user attempts to add an ignored file to + Shown when the user attempts to add an ignored file to the index. amWorkDir:: - Advice that shows the location of the patch file when - linkgit:git-am[1] fails to apply it. + Shown when linkgit:git-am[1] fails to apply a patch + file: tell the location of the file. ambiguousFetchRefspec:: - Advice shown when a fetch refspec for multiple remotes maps to + Shown when a fetch refspec for multiple remotes maps to the same remote-tracking branch namespace and causes branch tracking set-up to fail. checkoutAmbiguousRemoteBranchName:: - Advice shown when the argument to + Shown when the argument to linkgit:git-checkout[1] and linkgit:git-switch[1] ambiguously resolves to a remote tracking branch on more than one remote in @@ -33,31 +33,31 @@ advice.*:: to be used by default in some situations where this advice would be printed. commitBeforeMerge:: - Advice shown when linkgit:git-merge[1] refuses to + Shown when linkgit:git-merge[1] refuses to merge to avoid overwriting local changes. detachedHead:: - Advice shown when you used + Shown when the user uses linkgit:git-switch[1] or linkgit:git-checkout[1] - to move to the detached HEAD state, to instruct how to + to move to the detached HEAD state; instruct how to create a local branch after the fact. diverging:: - Advice shown when a fast-forward is not possible. + Shown when a fast-forward is not possible. fetchShowForcedUpdates:: - Advice shown when linkgit:git-fetch[1] takes a long time + Shown when linkgit:git-fetch[1] takes a long time to calculate forced updates after ref updates, or to warn that the check is disabled. forceDeleteBranch:: - Advice shown when a user tries to delete a not fully merged + Shown when the user tries to delete a not fully merged branch without the force option set. ignoredHook:: - Advice shown if a hook is ignored because the hook is not + Shown when a hook is ignored because the hook is not set as executable. implicitIdentity:: - Advice on how to set your identity configuration when - your information is guessed from the system username and - domain name. + Shown when the user's information is guessed from the + system username and domain name: tell the user how to + set their identity configuration. nestedTag:: - Advice shown if a user attempts to recursively tag a tag object. + Shown when a user attempts to recursively tag a tag object. pushAlreadyExists:: Shown when linkgit:git-push[1] rejects an update that does not qualify for fast-forwarding (e.g., a tag.) @@ -71,12 +71,12 @@ advice.*:: object that is not a commit-ish, or make the remote ref point at an object that is not a commit-ish. pushNonFFCurrent:: - Advice shown when linkgit:git-push[1] fails due to a + Shown when linkgit:git-push[1] fails due to a non-fast-forward update to the current branch. pushNonFFMatching:: - Advice shown when you ran linkgit:git-push[1] and pushed - 'matching refs' explicitly (i.e. you used ':', or - specified a refspec that isn't your current branch) and + Shown when the user ran linkgit:git-push[1] and pushed + 'matching refs' explicitly (i.e. used ':', or + specified a refspec that isn't the current branch) and it resulted in a non-fast-forward error. pushRefNeedsUpdate:: Shown when linkgit:git-push[1] rejects a forced update of @@ -95,17 +95,17 @@ advice.*:: 'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate' simultaneously. resetNoRefresh:: - Advice to consider using the `--no-refresh` option to - linkgit:git-reset[1] when the command takes more than 2 seconds - to refresh the index after reset. + Shown when linkgit:git-reset[1] takes more than 2 + seconds to refresh the index after reset: tell the user + that they can use the `--no-refresh` option. resolveConflict:: - Advice shown by various commands when conflicts + Shown by various commands when conflicts prevent the operation from being performed. rmHints:: - In case of failure in the output of linkgit:git-rm[1], - show directions on how to proceed from the current state. + Shown on failure in the output of linkgit:git-rm[1]: + give directions on how to proceed from the current state. sequencerInUse:: - Advice shown when a sequencer command is already in progress. + Shown when a sequencer command is already in progress. skippedCherryPicks:: Shown when linkgit:git-rebase[1] skips a commit that has already been cherry-picked onto the upstream branch. @@ -123,27 +123,27 @@ advice.*:: by linkgit:git-switch[1] or linkgit:git-checkout[1] when switching branches. statusUoption:: - Advise to consider using the `-u` option to linkgit:git-status[1] - when the command takes more than 2 seconds to enumerate untracked - files. + Shown when linkgit:git-status[1] takes more than 2 + seconds to enumerate untracked files: consider using the + `-u` option. submoduleAlternateErrorStrategyDie:: - Advice shown when a submodule.alternateErrorStrategy option + Shown when a submodule.alternateErrorStrategy option configured to "die" causes a fatal error. submodulesNotUpdated:: - Advice shown when a user runs a submodule command that fails + Shown when a user runs a submodule command that fails because `git submodule update --init` was not run. suggestDetachingHead:: - Advice shown when linkgit:git-switch[1] refuses to detach HEAD + Shown when linkgit:git-switch[1] refuses to detach HEAD without the explicit `--detach` option. updateSparsePath:: - Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1] + Shown when either linkgit:git-add[1] or linkgit:git-rm[1] is asked to update index entries outside the current sparse checkout. waitingForEditor:: - Print a message to the terminal whenever Git is waiting for - editor input from the user. + Shown when Git is waiting for editor input. Relevant + when e.g. the editor is not launched inside the terminal. worktreeAddOrphan:: - Advice shown when a user tries to create a worktree from an - invalid reference, to instruct how to create a new unborn + Shown when the user tries to create a worktree from an + invalid reference: instruct how to create a new unborn branch instead. --
1. Use “shown” instead of “advice shown” • “advice” is implied and a bit repetitive 2. Use “when” instead of “if” 3. Lead with “Shown when” and end the entry with the effect it has, where applicable 4. Use “the user” instead of “a user” or “you” 5. detachedHead: connect clause with a semicolon to make the sentence flow better in this new context 6. implicitIdentity: rewrite description in order to lead with *when* the advice is shown (see point (3)) 7. Prefer the present tense (with the exception of pushNonFFMatching) 8. Use a colon to connect the last clause instead of a comma 9. waitingForEditor: give example of relevance in this new context 10. pushUpdateRejected: exception to the above principles Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> --- Notes (series): Maybe the style that we eventually agree on should be documented outside the commit log? Documentation/config/advice.txt | 80 ++++++++++++++++----------------- 1 file changed, 40 insertions(+), 40 deletions(-)