diff mbox series

[v2] name-rev: make --stdin hidden

Message ID pull.1225.v2.git.git.1683314270964.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v2] name-rev: make --stdin hidden | expand

Commit Message

John Cai May 5, 2023, 7:17 p.m. UTC
From: John Cai <johncai86@gmail.com>

In 34ae3b70 (name-rev: deprecate --stdin in favor of --annotate-stdin),
we renamed --stdin to --annotate-stdin for the sake of a clearer name
for the option, and added text that indicates --stdin is deprecated. The
next step is to hide --stdin completely.

Make the option hidden. Also, update documentation to remove all
mentions of --stdin.

Signed-off-by: "John Cai" <johncai86@gmail.com>
---
    name-rev: make --stdin hidden
    
    Now that --stdin has been deprecated for several releases, the next step
    of replacing name-rev --stdin with --annotate-stdin is to make --stdin
    hidden. This patch also updates documentation to get rid of any mention
    of --stdin.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1225%2Fjohn-cai%2Fjc%2Fhide-name-rev-stdin-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1225/john-cai/jc/hide-name-rev-stdin-v2
Pull-Request: https://github.com/git/git/pull/1225

Range-diff vs v1:

 1:  32c8db2f03a ! 1:  904cd2c3572 name-rev: make --stdin hidden
     @@ builtin/name-rev.c: int cmd_name_rev(int argc, const char **argv, const char *pr
       				   N_("ignore refs matching <pattern>")),
       		OPT_GROUP(""),
       		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
     --		OPT_BOOL(0, "stdin", &transform_stdin, N_("deprecated: use annotate-stdin instead")),
     +-		OPT_BOOL(0, "stdin", &transform_stdin, N_("deprecated: use --annotate-stdin instead")),
      +		OPT_BOOL_F(0,
      +			   "stdin",
      +			   &transform_stdin,
     -+			   N_("deprecated: use annotate-stdin instead"),
     ++			   N_("deprecated: use --annotate-stdin instead"),
      +			   PARSE_OPT_HIDDEN),
       		OPT_BOOL(0, "annotate-stdin", &annotate_stdin, N_("annotate text from stdin")),
       		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),


 Documentation/git-name-rev.txt | 8 ++------
 builtin/name-rev.c             | 6 +++++-
 2 files changed, 7 insertions(+), 7 deletions(-)


base-commit: 69c786637d7a7fe3b2b8f7d989af095f5f49c3a8

Comments

Eric Sunshine May 5, 2023, 7:31 p.m. UTC | #1
On Fri, May 5, 2023 at 3:19 PM John Cai via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> In 34ae3b70 (name-rev: deprecate --stdin in favor of --annotate-stdin),
> we renamed --stdin to --annotate-stdin for the sake of a clearer name
> for the option, and added text that indicates --stdin is deprecated. The
> next step is to hide --stdin completely.
>
> Make the option hidden. Also, update documentation to remove all
> mentions of --stdin.

Eradicating all mention of --stdin from the documentation makes it
more hostile for end-users, doesn't it? If someone runs across --stdin
in a blog post or in some in-the-wild script, then this makes it more
difficult to learn what the option does. In other such cases, rather
than purging all mention from documentation, we've instead mentioned
the deprecated option only as a minor aside of the option which
replaces it. For instance:

    --annotate-stdin::
        Transform stdin by ... omitting $hex altogether.
        `--stdin` is a deprecated synonym.

> Signed-off-by: "John Cai" <johncai86@gmail.com>
Junio C Hamano May 5, 2023, 7:37 p.m. UTC | #2
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> In 34ae3b70 (name-rev: deprecate --stdin in favor of --annotate-stdin),
> we renamed --stdin to --annotate-stdin for the sake of a clearer name
> for the option, and added text that indicates --stdin is deprecated. The
> next step is to hide --stdin completely.
>
> Make the option hidden. Also, update documentation to remove all
> mentions of --stdin.
>
> Signed-off-by: "John Cai" <johncai86@gmail.com>
> ---
>     name-rev: make --stdin hidden
>     
>     Now that --stdin has been deprecated for several releases, the next step
>     of replacing name-rev --stdin with --annotate-stdin is to make --stdin
>     hidden. This patch also updates documentation to get rid of any mention
>     of --stdin.

Nice.  It has been a year, and I agree that it is about time.

Thanks for not forgetting about the topic.

>      -+			   N_("deprecated: use annotate-stdin instead"),
>      ++			   N_("deprecated: use --annotate-stdin instead"),

And of course this one is a very nice touch, relative to the
previous round.

>       +			   PARSE_OPT_HIDDEN),

> diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
> index ec8a27ce8bf..5f196c03708 100644
> --- a/Documentation/git-name-rev.txt
> +++ b/Documentation/git-name-rev.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git name-rev' [--tags] [--refs=<pattern>]
> -	       ( --all | --stdin | <commit-ish>... )
> +	       ( --all | --annotate-stdin | <commit-ish>... )
>  
>  DESCRIPTION
>  -----------
> @@ -70,10 +70,6 @@ The full name after substitution is master,
>  while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
>  -----------
>  
> ---stdin::
> -	This option is deprecated in favor of 'git name-rev --annotate-stdin'.
> -	They are functionally equivalent.
> -
>  --name-only::
>  	Instead of printing both the SHA-1 and the name, print only
>  	the name.  If given with --tags the usual tag prefix of

I agree with the main thrust of the change, but I am not sure if it
is a good idea to "completely" remove the mention.

Even after we stop talking about it, people find old scriptlets that
use "name-rev --stdin" from various random places on the Internet,
and wonder if they are buggy.  I wonder if having something like
this for a year or two may help?  We would need to scan for "was
called" and decide to clean them up once in a while, of course.

Will queue as is.  Thanks.

 Documentation/git-name-rev.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git c/Documentation/git-name-rev.txt w/Documentation/git-name-rev.txt
index 5f196c0370..1173deae57 100644
--- c/Documentation/git-name-rev.txt
+++ w/Documentation/git-name-rev.txt
@@ -46,7 +46,8 @@ OPTIONS
 	Transform stdin by substituting all the 40-character SHA-1
 	hexes (say $hex) with "$hex ($rev_name)".  When used with
 	--name-only, substitute with "$rev_name", omitting $hex
-	altogether.
+	altogether.  This option was called `--stdin` in older
+	versions of Git.
 +
 For example:
 +
John Cai May 5, 2023, 9:42 p.m. UTC | #3
Hi Junio,

On 5 May 2023, at 15:37, Junio C Hamano wrote:

> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: John Cai <johncai86@gmail.com>
>>
>> In 34ae3b70 (name-rev: deprecate --stdin in favor of --annotate-stdin),
>> we renamed --stdin to --annotate-stdin for the sake of a clearer name
>> for the option, and added text that indicates --stdin is deprecated. The
>> next step is to hide --stdin completely.
>>
>> Make the option hidden. Also, update documentation to remove all
>> mentions of --stdin.
>>
>> Signed-off-by: "John Cai" <johncai86@gmail.com>
>> ---
>>     name-rev: make --stdin hidden
>>
>>     Now that --stdin has been deprecated for several releases, the next step
>>     of replacing name-rev --stdin with --annotate-stdin is to make --stdin
>>     hidden. This patch also updates documentation to get rid of any mention
>>     of --stdin.
>
> Nice.  It has been a year, and I agree that it is about time.
>
> Thanks for not forgetting about the topic.
>
>>      -+			   N_("deprecated: use annotate-stdin instead"),
>>      ++			   N_("deprecated: use --annotate-stdin instead"),
>
> And of course this one is a very nice touch, relative to the
> previous round.
>
>>       +			   PARSE_OPT_HIDDEN),
>
>> diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
>> index ec8a27ce8bf..5f196c03708 100644
>> --- a/Documentation/git-name-rev.txt
>> +++ b/Documentation/git-name-rev.txt
>> @@ -10,7 +10,7 @@ SYNOPSIS
>>  --------
>>  [verse]
>>  'git name-rev' [--tags] [--refs=<pattern>]
>> -	       ( --all | --stdin | <commit-ish>... )
>> +	       ( --all | --annotate-stdin | <commit-ish>... )
>>
>>  DESCRIPTION
>>  -----------
>> @@ -70,10 +70,6 @@ The full name after substitution is master,
>>  while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
>>  -----------
>>
>> ---stdin::
>> -	This option is deprecated in favor of 'git name-rev --annotate-stdin'.
>> -	They are functionally equivalent.
>> -
>>  --name-only::
>>  	Instead of printing both the SHA-1 and the name, print only
>>  	the name.  If given with --tags the usual tag prefix of
>
> I agree with the main thrust of the change, but I am not sure if it
> is a good idea to "completely" remove the mention.
>
> Even after we stop talking about it, people find old scriptlets that
> use "name-rev --stdin" from various random places on the Internet,
> and wonder if they are buggy.  I wonder if having something like
> this for a year or two may help?  We would need to scan for "was
> called" and decide to clean them up once in a while, of course.

Yeah, that's valid.

>
> Will queue as is.  Thanks.
>
>  Documentation/git-name-rev.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git c/Documentation/git-name-rev.txt w/Documentation/git-name-rev.txt
> index 5f196c0370..1173deae57 100644
> --- c/Documentation/git-name-rev.txt
> +++ w/Documentation/git-name-rev.txt
> @@ -46,7 +46,8 @@ OPTIONS
>  	Transform stdin by substituting all the 40-character SHA-1
>  	hexes (say $hex) with "$hex ($rev_name)".  When used with
>  	--name-only, substitute with "$rev_name", omitting $hex
> -	altogether.
> +	altogether.  This option was called `--stdin` in older
> +	versions of Git.
>  +
>  For example:
>  +

Sounds good to me. Will add this in

thanks!
John
Teng Long May 6, 2023, 12:37 p.m. UTC | #4
John Cai <johncai86@gmail.com> writes:

>-		OPT_BOOL(0, "stdin", &transform_stdin, N_("deprecated: use --annotate-stdin instead")),
>+		OPT_BOOL_F(0,
>+			   "stdin",
>+			   &transform_stdin,
>+			   N_("deprecated: use --annotate-stdin instead"),
>+			   PARSE_OPT_HIDDEN),
> 		OPT_BOOL(0, "annotate-stdin", &annotate_stdin, N_("annotate text from stdin")),
> 		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
> 		OPT_BOOL(0, "always",     &always,

It seems like there is an odd indent before "&always", of course, it's
not introduced by this patch.

Thanks.
diff mbox series

Patch

diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index ec8a27ce8bf..5f196c03708 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -10,7 +10,7 @@  SYNOPSIS
 --------
 [verse]
 'git name-rev' [--tags] [--refs=<pattern>]
-	       ( --all | --stdin | <commit-ish>... )
+	       ( --all | --annotate-stdin | <commit-ish>... )
 
 DESCRIPTION
 -----------
@@ -70,10 +70,6 @@  The full name after substitution is master,
 while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
 -----------
 
---stdin::
-	This option is deprecated in favor of 'git name-rev --annotate-stdin'.
-	They are functionally equivalent.
-
 --name-only::
 	Instead of printing both the SHA-1 and the name, print only
 	the name.  If given with --tags the usual tag prefix of
@@ -107,7 +103,7 @@  Now you are wiser, because you know that it happened 940 revisions before v0.99.
 Another nice thing you can do is:
 
 ------------
-% git log | git name-rev --stdin
+% git log | git name-rev --annotate-stdin
 ------------
 
 GIT
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 593f0506a10..4d15a23fc4d 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -573,7 +573,11 @@  int cmd_name_rev(int argc, const char **argv, const char *prefix)
 				   N_("ignore refs matching <pattern>")),
 		OPT_GROUP(""),
 		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
-		OPT_BOOL(0, "stdin", &transform_stdin, N_("deprecated: use --annotate-stdin instead")),
+		OPT_BOOL_F(0,
+			   "stdin",
+			   &transform_stdin,
+			   N_("deprecated: use --annotate-stdin instead"),
+			   PARSE_OPT_HIDDEN),
 		OPT_BOOL(0, "annotate-stdin", &annotate_stdin, N_("annotate text from stdin")),
 		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
 		OPT_BOOL(0, "always",     &always,