name-rev: deprecate --stdin in favor of --annotate-text
PATCH 1/1 name-rev: deprecate --stdin in favor of --annotate-text
From: John Cai <[email protected]>
Introduce a --annotate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-text.
This change adds a warning to --stdin warning that it will be removed in the future.
---
name-rev: deprecate --stdin in favor of --anotate-text
Introduce a --anontate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, the goal is to rename it to --anotate-text.
This is the first step in a process of eventually fully deprecating --stdin. This change also adds a warning to --stdin warning that it will be removed in the future.
See https://lore.kernel.org/git/[email protected]/ for discussion.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1171%2Fjohn-cai%2Fjc%2Fdeprecate-name-rev-stdin-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1171/john-cai/jc/deprecate-name-rev-stdin-v1
Pull-Request: https://github.com/git/git/pull/1171
Documentation/git-name-rev.txt | 29 ++++++++++++++++++++++++++++- builtin/name-rev.c | 17 +++++++++++++---- 2 files changed, 41 insertions(+), 5 deletions(-)
Documentation/git-name-rev.txt +28 -1
@@ -43,10 +43,37 @@ OPTIONS List all commits reachable from all refs --stdin:: + This option is deprecated in favor of 'git name-rev --annotate-text'. + They are functionally equivalent. + +--annotate-text:: 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. Intended for the scripter's use. + altogether. + + For example: ++ +---------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-text < sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 +(master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-text < sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name is master, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad +---------- --name-only:: Instead of printing both the SHA-1 and the name, print only
builtin/name-rev.c +13 -4
@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_text =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),
@@ -539,6 +539,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)OPT_GROUP ("" ),OPT_BOOL (0 ,"all" , &all,N_ ("list all commits reachable from all refs" )),OPT_BOOL (0 ,"stdin" , &transform_stdin,N_ ("read from stdin" )), +OPT_BOOL (0 ,"annotate-text" , &annotate_text,N_ ("annotate text text from stdin" )),OPT_BOOL (0 ,"undefined" , &allow_undefined,N_ ("allow to print `undefined` names (default)" )),OPT_BOOL (0 ,"always" , &always, N_("show abbreviated commit object as fallback")),
@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-text instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_text =1 ; + } + +if (all + annotate_text + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_text) cutoff =0 ;for (; argc; argc--, argv++) {
@@ -613,7 +622,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { +if (annotate_text) {char buffer[2048 ];while (!feof (stdin)) {
base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907
Signature · 1 line
gitgitgadget
"John Cai via GitGitGadget" <[email protected]> writes:
As name-rev is Dscho's brainchild, I think it is benefitial to ask input from him, so I added an address to the CC: list.
Quoted 9 lines
From: John Cai <[email protected]>
Introduce a --annotate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-text.
This change adds a warning to --stdin warning that it will be removed in the future.
I know I've suggested the name, but 'text' in --annotate-text is a low value word in an option name where every byte is precious. "Annotate" is very good to convey what is done to the object of the verb, but "text" stresses the wrong thing (we do not annotate binary,o we annotate text) without saying where the text comes from (i.e. standard input). Perhaps "--annotate-stdin" would be a much better name.
I agree that letting "--stdin" to deviate so much from an emulation of "xargs git name-rev" was indeed a mistake that caused the confusion that led to the other thread. If the mode had a better name from the day 1, the thread would have been avoided.
What I am not sure about is how much this transition would hurt existing users and scripts.
Documentation/git-name-rev.txt OPTIONS · line 64
Quoted 10 lines
Documentation/git-name-rev.txt +10
-0+ For example: ++ +---------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-text < sample.txt
Lose SP between the redirection operator '<' and its file 'sample.txt'.
Documentation/git-name-rev.txt OPTIONS · line 71
Quoted 7 lines
Documentation/git-name-rev.txt +7
-0+ +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 +(master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-text < sample.txt
Ditto.
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 563
Quoted 5 lines
builtin/name-rev.c +5
-0+if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-text instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_text =1 ;
I guess that is sensible. To squelch the warning, they can switch to the new option.
Quoted 12 lines
builtin/name-rev.c +4
-1+ } + +if (all + annotate_text + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_text) cutoff =0 ;for (; argc; argc--, argv++) {
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 628
Quoted 8 lines
builtin/name-rev.c +1
-1@@ -613,7 +622,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { +if (annotate_text) {char buffer[2048 ];while (!feof (stdin)) {
Not a suggestion to change anything in this patch, but just an observation.
- If the mode is useful enough for many users, certainly somebody would have rewritten this loop to lift the line-length limit, but nobody noticed and complained about this 2k limit for the past 17 years. I am not sure if that is an indication that nobody uses the option in its current form.
- A low hanging fruit, if we do not go full strbuf_getline(), at least we should narrow the scope of buffer[] array.
Quoted 91 lines · Junio C Hamano
"John Cai via GitGitGadget" <[email protected]> writes:
As name-rev is Dscho's brainchild, I think it is benefitial to ask input from him, so I added an address to the CC: list.
Quoted 9 lines
From: John Cai <[email protected]>
Introduce a --annotate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-text.
This change adds a warning to --stdin warning that it will be removed in the future.
I know I've suggested the name, but 'text' in --annotate-text is a low value word in an option name where every byte is precious. "Annotate" is very good to convey what is done to the object of the verb, but "text" stresses the wrong thing (we do not annotate binary,o we annotate text) without saying where the text comes from (i.e. standard input). Perhaps "--annotate-stdin" would be a much better name.
I agree that letting "--stdin" to deviate so much from an emulation of "xargs git name-rev" was indeed a mistake that caused the confusion that led to the other thread. If the mode had a better name from the day 1, the thread would have been avoided.
What I am not sure about is how much this transition would hurt existing users and scripts.
Documentation/git-name-rev.txt OPTIONS · line 64
Quoted 10 lines
Documentation/git-name-rev.txt +10
-0+ For example: ++ +---------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-text < sample.txtLose SP between the redirection operator '<' and its file 'sample.txt'.
Documentation/git-name-rev.txt OPTIONS · line 71
Quoted 7 lines
Documentation/git-name-rev.txt +7
-0+ +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 +(master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-text < sample.txtDitto.
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 563
Quoted 5 lines
builtin/name-rev.c +5
-0+if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-text instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_text =1 ;I guess that is sensible. To squelch the warning, they can switch to the new option.
Quoted 12 lines
builtin/name-rev.c +3
-0+ } + +if (all + annotate_text + !!argc >1 ) {error("Specify either a list, or --all, not both!"); usage_with_options(name_rev_usage, opts); }builtin/name-rev.c +1
-1-if (all || transform_stdin) +if (all || annotate_text)cutoff = 0; for (; argc; argc--, argv++) {builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 628
Quoted 8 lines
builtin/name-rev.c +1
-1@@ -613,7 +622,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { +if (annotate_text) {char buffer[2048 ];while (!feof (stdin)) {Not a suggestion to change anything in this patch, but just an observation.
Quoted 5 lines
- If the mode is useful enough for many users, certainly somebody would have rewritten this loop to lift the line-length limit, but nobody noticed and complained about this 2k limit for the past 17 years. I am not sure if that is an indication that nobody uses the option in its current form.
Would this also mean that deprecating --stdin wouldn’t be **too** disruptive to existing users and scripts?
Quoted 3 lines
- A low hanging fruit, if we do not go full
strbuf_getline(), at least we should narrow the scope of buffer[] array.
v2 cover name-rev: deprecate --stdin in favor of --anotate-text
In reply to John Cai via GitGitGadgetIntroduce a --anontate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, the goal is to rename it to --anotate-text.
This is the first step in a process of eventually fully deprecating --stdin. This change also adds a warning to --stdin warning that it will be removed in the future.
See https://lore.kernel.org/git/[email protected]/ for discussion.
changes since v1 (thanks to Junio's review):
* s/annotate_text/annotate_stdin
* add a commit to replace the 2048 size buffer with a strbuf and use strbuf_getwholeline to get lines from stdin
* fixed formatting bugs in documentation udpates.
John Cai (2):
name-rev: deprecate --stdin in favor of --annotate-text name-rev.c: use strbuf_getline instead of limited size buffer
Documentation/git-name-rev.txt | 29 ++++++++++++++++++++++++++++- builtin/name-rev.c | 27 +++++++++++++++++---------- 2 files changed, 45 insertions(+), 11 deletions(-)
base-commit: 55b058a8bbcc54bd93c733035c995abc7967e539
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1171%2Fjohn-cai%2Fjc%2Fdeprecate-name-rev-stdin-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1171/john-cai/jc/deprecate-name-rev-stdin-v2
Pull-Request: https://github.com/git/git/pull/1171
Range-diff vs v1:
1: b83d9422d0c ! 1: e8063284b4d name-rev: deprecate --stdin in favor of --annotate-text
@@ Documentation/git-name-rev.txt: OPTIONS
List all commits reachable from all refs
--stdin::
-+ This option is deprecated in favor of 'git name-rev --annotate-text'.
++ This option is deprecated in favor of 'git name-rev --annotate-stdin'.
+ They are functionally equivalent.
+
-+--annotate-text::
++--annotate-stdin::
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
@@ Documentation/git-name-rev.txt: OPTIONS
+The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907,
+while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
+
-+$ git name-rev --annotate-text < sample.txt
++$ git name-rev --annotate-stdin <sample.txt
+
+An abbreviated revision 2ae0a9cb82 will not be substituted.
+The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907
+(master),
+while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
+
-+$ git name-rev --name-only --annotate-text < sample.txt
++$ git name-rev --name-only --annotate-stdin <sample.txt
+
+An abbreviated revision 2ae0a9cb82 will not be substituted.
+The full name is master,
@@ builtin/name-rev.c: static void name_rev_line(char *p, struct name_ref_data *dat
{
struct object_array revs = OBJECT_ARRAY_INIT;
- int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
-+ int all = 0, annotate_text = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
++ int all = 0, annotate_stdin = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
struct option opts[] = {
OPT_BOOL(0, "name-only", &data.name_only, N_("print only ref-based names (no object names)")),
@@ builtin/name-rev.c: int cmd_name_rev(int argc, const char **argv, const char *pr
OPT_GROUP(""),
OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),
-+ OPT_BOOL(0, "annotate-text", &annotate_text, N_("annotate text text from stdin")),
++ 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,
N_("show abbreviated commit object as fallback")),
@@ builtin/name-rev.c: int cmd_name_rev(int argc, const char **argv, const char *pr
- if (all + transform_stdin + !!argc > 1) {
+
+ if (transform_stdin) {
-+ warning("--stdin is deprecated. Please use --annotate-text instead, "
++ warning("--stdin is deprecated. Please use --annotate-stdin instead, "
+ "which is functionally equivalent.\n"
+ "This option will be removed in a future release.");
-+ annotate_text = 1;
++ annotate_stdin = 1;
+ }
+
-+ if (all + annotate_text + !!argc > 1) {
++ if (all + annotate_stdin + !!argc > 1) {
error("Specify either a list, or --all, not both!");
usage_with_options(name_rev_usage, opts);
}
- if (all || transform_stdin)
-+ if (all || annotate_text)
++ if (all || annotate_stdin)
cutoff = 0;
for (; argc; argc--, argv++) {
@@ builtin/name-rev.c: int cmd_name_rev(int argc, const char **argv, const char *pr
name_tips();
- if (transform_stdin) {
-+ if (annotate_text) {
- char buffer[2048];
+- char buffer[2048];
++ if (annotate_stdin) {
++ struct strbuf sb = STRBUF_INIT;
while (!feof(stdin)) {
+ char *p = fgets(buffer, sizeof(buffer), stdin); -: ----------- > 2: 4636e27f53e name-rev.c: use strbuf_getline instead of limited size buffer
Signature · 1 line
gitgitgadget
PATCH v2 1/2 name-rev: deprecate --stdin in favor of --annotate-text
From: John Cai <[email protected]>
Introduce a --annotate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-text.
This change adds a warning to --stdin warning that it will be removed in the future.
--- Documentation/git-name-rev.txt | 29 ++++++++++++++++++++++++++++- builtin/name-rev.c | 19 ++++++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-)
Documentation/git-name-rev.txt +28 -1
@@ -43,10 +43,37 @@ OPTIONS List all commits reachable from all refs --stdin:: + This option is deprecated in favor of 'git name-rev --annotate-stdin'. + They are functionally equivalent. + +--annotate-stdin:: 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. Intended for the scripter's use. + altogether. + + For example: ++ +---------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 +(master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name is master, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad +---------- --name-only:: Instead of printing both the SHA-1 and the name, print only
builtin/name-rev.c +14 -5
@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_stdin =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),
@@ -539,6 +539,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)OPT_GROUP ("" ),OPT_BOOL (0 ,"all" , &all,N_ ("list all commits reachable from all refs" )),OPT_BOOL (0 ,"stdin" , &transform_stdin,N_ ("read from stdin" )), +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, N_("show abbreviated commit object as fallback")),
@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-stdin instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_stdin =1 ; + } + +if (all + annotate_stdin + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_stdin) cutoff =0 ;for (; argc; argc--, argv++) {
@@ -613,8 +622,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { -char buffer[2048 ]; +if (annotate_stdin) { +struct strbuf sb =STRBUF_INIT ;while (!feof (stdin)) {char *p =fgets (buffer,sizeof (buffer), stdin);
Signature · 1 line
gitgitgadget
PATCH v2 2/2 name-rev.c: use strbuf_getline instead of limited size buffer
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
Using a buffer limited to 2048 is unnecessarily limiting. Switch to using a string buffer to read in stdin for annotation.
--- builtin/name-rev.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
builtin/name-rev.c +3 -5
@@ -625,12 +625,10 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)if (annotate_stdin) {struct strbuf sb =STRBUF_INIT ; -while (!feof (stdin)) { -char *p =fgets (buffer,sizeof (buffer), stdin); -if (!p) -break ; -name_rev_line (p, &data); +while (strbuf_getwholeline (&sb, stdin,'\n' ) !=EOF ) { +name_rev_line (sb.buf, &data); } +strbuf_release (&sb); }else if (all) {int i, max;
Signature · 1 line
gitgitgadget
Quoted 9 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Introduce a --annotate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-text.
This change adds a warning to --stdin warning that it will be removed in the future.
The above (including the title) uses --annotate-TEXT; the documentation and the code seems to use --annotate-STDIN.
Quoted 6 lines
Signed-off-by "John Cai" <[email protected]>--- Documentation/git-name-rev.txt | 29 ++++++++++++++++++++++++++++- builtin/name-rev.c | 19 ++++++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-)
We probably would want to fix existing tests that use --stdin for its originally intended purpose to use its new spelling instead, and a single new test to ensure that use of --stdin gives a warning as expected.
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 629
Quoted 11 lines
builtin/name-rev.c +2
-2@@ -613,8 +622,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { -char buffer[2048 ]; +if (annotate_stdin) { +struct strbuf sb =STRBUF_INIT ;while (!feof (stdin)) {char *p =fgets (buffer,sizeof (buffer), stdin);
This block probably would not compile well.
Quoted 11 lines · John Cai
Quoted 8 lines
Not a suggestion to change anything in this patch, but just an observation.
- If the mode is useful enough for many users, certainly somebody would have rewritten this loop to lift the line-length limit, but nobody noticed and complained about this 2k limit for the past 17 years. I am not sure if that is an indication that nobody uses the option in its current form.
Would this also mean that deprecating --stdin wouldn’t be **too** disruptive to existing users and scripts?
That probably is a lot of wishful thinking. Most likely, it is an indication that most text input files that go into the command have no overly long lines ;-) Loss of or change in behaviour of the command would be disruptive for the users who are content with the limitation that they cannot feed lines wider than 2k.
Hi Junio,
Quoted 4 lines · Junio C Hamano
"John Cai via GitGitGadget" <[email protected]> writes:
As name-rev is Dscho's brainchild, I think it is benefitial to ask input from him, so I added an address to the CC: list.
Thank you!
Quoted 10 lines
Quoted 9 lines
From: John Cai <[email protected]>
Introduce a --annotate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-text.
This change adds a warning to --stdin warning that it will be removed in the future.
Quoted 7 lines
I know I've suggested the name, but 'text' in --annotate-text is a low value word in an option name where every byte is precious. "Annotate" is very good to convey what is done to the object of the verb, but "text" stresses the wrong thing (we do not annotate binary,o we annotate text) without saying where the text comes from (i.e. standard input). Perhaps "--annotate-stdin" would be a much better name.
I agree that --annotate-stdin is better, especially since people who might look for a replacement for --stdin in the documentation are much more likely to find what they're looking for.
In addition to this, I also agree with Junio on the deprecation needing to be done carefully.
Ciao,
Dscho
v3 cover name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetIntroduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, the goal is to rename it to --anotate-stdin.
This is the first step in a process of eventually fully deprecating --stdin. This change also adds a warning to --stdin warning that it will be removed in the future.
See https://lore.kernel.org/git/[email protected]/ for discussion.
changes since v2:
* updated title and cover letter with --annotate-stdin
* s/--stdin/--annotate-stdin in tests that call name-rev
* added a test to ensure correct warning is shown when --stdin is passed
John Cai (2):
name-rev: deprecate --stdin in favor of --annotate-text name-rev.c: use strbuf_getline instead of limited size buffer
Documentation/git-name-rev.txt | 29 +++++++++++++++++++++++++++- builtin/name-rev.c | 27 ++++++++++++++++---------- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------- t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 70 insertions(+), 30 deletions(-)
base-commit: c8b2ade48c204690119936ada89cd938c476c5c2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1171%2Fjohn-cai%2Fjc%2Fdeprecate-name-rev-stdin-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1171/john-cai/jc/deprecate-name-rev-stdin-v3
Pull-Request: https://github.com/git/git/pull/1171
Range-diff vs v2:
1: e8063284b4d ! 1: 55ec2a5fa3e name-rev: deprecate --stdin in favor of --annotate-text
@@ builtin/name-rev.c: int cmd_name_rev(int argc, const char **argv, const char *pr
while (!feof(stdin)) {
char *p = fgets(buffer, sizeof(buffer), stdin);
+
+ ## t/t3412-rebase-root.sh ##
+@@ t/t3412-rebase-root.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+ log_with_names () {
+ git rev-list --topo-order --parents --pretty="tformat:%s" HEAD |
+- git name-rev --stdin --name-only --refs=refs/heads/$1
++ git name-rev --annotate-stdin --name-only --refs=refs/heads/$1
+ }
+
+
+
+ ## t/t4202-log.sh ##
+@@ t/t4202-log.sh: EOF
+
+ test_expect_success 'log --graph with full output' '
+ git log --graph --date-order --pretty=short |
+- git name-rev --name-only --stdin |
++ git name-rev --name-only --annotate-stdin |
+ sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual &&
+ test_cmp expect actual
+ '
+
+ ## t/t6007-rev-list-cherry-pick-file.sh ##
+@@ t/t6007-rev-list-cherry-pick-file.sh: EOF
+
+ test_expect_success '--left-right' '
+ git rev-list --left-right B...C > actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" \
+ < actual > actual.named &&
+ test_cmp expect actual.named
+ '
+@@ t/t6007-rev-list-cherry-pick-file.sh: EOF
+
+ test_expect_success '--cherry-pick bar does not come up empty' '
+ git rev-list --left-right --cherry-pick B...C -- bar > actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" \
+ < actual > actual.named &&
+ test_cmp expect actual.named
+ '
+
+ test_expect_success 'bar does not come up empty' '
+ git rev-list --left-right B...C -- bar > actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" \
+ < actual > actual.named &&
+ test_cmp expect actual.named
+ '
+@@ t/t6007-rev-list-cherry-pick-file.sh: EOF
+
+ test_expect_success '--cherry-pick bar does not come up empty (II)' '
+ git rev-list --left-right --cherry-pick F...E -- bar > actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" \
+ < actual > actual.named &&
+ test_cmp expect actual.named
+ '
+
+ test_expect_success 'name-rev multiple --refs combine inclusive' '
+ git rev-list --left-right --cherry-pick F...E -- bar >actual &&
+- git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \
+ <actual >actual.named &&
+ test_cmp expect actual.named
+ '
+@@ t/t6007-rev-list-cherry-pick-file.sh: EOF
+ test_expect_success 'name-rev --refs excludes non-matched patterns' '
+ git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect &&
+ git rev-list --left-right --cherry-pick F...E -- bar >actual &&
+- git name-rev --stdin --name-only --refs="*tags/F" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/F" \
+ <actual >actual.named &&
+ test_cmp expect actual.named
+ '
+@@ t/t6007-rev-list-cherry-pick-file.sh: EOF
+ test_expect_success 'name-rev --exclude excludes matched patterns' '
+ git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect &&
+ git rev-list --left-right --cherry-pick F...E -- bar >actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \
+ <actual >actual.named &&
+ test_cmp expect actual.named
+ '
+
+ test_expect_success 'name-rev --no-refs clears the refs list' '
+ git rev-list --left-right --cherry-pick F...E -- bar >expect &&
+- git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \
+ <expect >actual &&
+ test_cmp expect actual
+ '
+@@ t/t6007-rev-list-cherry-pick-file.sh: EOF
+
+ test_expect_success '--cherry-mark' '
+ git rev-list --cherry-mark F...E -- bar > actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" \
+ < actual > actual.named &&
+ test_cmp expect actual.named
+ '
+@@ t/t6007-rev-list-cherry-pick-file.sh: EOF
+
+ test_expect_success '--cherry-mark --left-right' '
+ git rev-list --cherry-mark --left-right F...E -- bar > actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" \
+ < actual > actual.named &&
+ test_cmp expect actual.named
+ '
+@@ t/t6007-rev-list-cherry-pick-file.sh: EOF
+
+ test_expect_success '--cherry-pick --right-only' '
+ git rev-list --cherry-pick --right-only F...E -- bar > actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" \
+ < actual > actual.named &&
+ test_cmp expect actual.named
+ '
+
+ test_expect_success '--cherry-pick --left-only' '
+ git rev-list --cherry-pick --left-only E...F -- bar > actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" \
+ < actual > actual.named &&
+ test_cmp expect actual.named
+ '
+@@ t/t6007-rev-list-cherry-pick-file.sh: EOF
+
+ test_expect_success '--cherry' '
+ git rev-list --cherry F...E -- bar > actual &&
+- git name-rev --stdin --name-only --refs="*tags/*" \
++ git name-rev --annotate-stdin --name-only --refs="*tags/*" \
+ < actual > actual.named &&
+ test_cmp expect actual.named
+ '
+
+ ## t/t6012-rev-list-simplify.sh ##
+@@ t/t6012-rev-list-simplify.sh: note () {
+ }
+
+ unnote () {
+- git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g"
++ git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g"
+ }
+
+ #
+
+ ## t/t6111-rev-list-treesame.sh ##
+@@ t/t6111-rev-list-treesame.sh: note () {
+ }
+
+ unnote () {
+- git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g"
++ git name-rev --tags --annotate-stdin | \
++ sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g"
+ }
+
+ test_expect_success setup '
+
+ ## t/t6120-describe.sh ##
+@@ t/t6120-describe.sh: test_expect_success 'name-rev --all' '
+ test_cmp expect actual
+ '
+
+-test_expect_success 'name-rev --stdin' '
++test_expect_success 'name-rev --annotate-stdin' '
+ >expect.unsorted &&
+ for rev in $(git rev-list --all)
+ do
+@@ t/t6120-describe.sh: test_expect_success 'name-rev --stdin' '
+ echo "$rev ($name)" >>expect.unsorted || return 1
+ done &&
+ sort <expect.unsorted >expect &&
+- git rev-list --all | git name-rev --stdin >actual.unsorted &&
++ git rev-list --all | git name-rev --annotate-stdin >actual.unsorted &&
+ sort <actual.unsorted >actual &&
+ test_cmp expect actual
+ '
+
++test_expect_success 'name-rev --stdin deprecated' "
++ git rev-list --all | git name-rev --stdin 2>actual &&
++ grep -E 'warning: --stdin is deprecated' actual
++"
++
+ test_expect_success 'describe --contains with the exact tags' '
+ echo "A^0" >expect &&
+ tag_object=$(git rev-parse refs/tags/A) &&
2: 4636e27f53e = 2: e4bd09ccf75 name-rev.c: use strbuf_getline instead of limited size buffer
Signature · 1 line
gitgitgadget
PATCH v3 1/2 name-rev: deprecate --stdin in favor of --annotate-text
From: John Cai <[email protected]>
Introduce a --annotate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-text.
This change adds a warning to --stdin warning that it will be removed in the future.
--- Documentation/git-name-rev.txt | 29 +++++++++++++++++++++++++++- builtin/name-rev.c | 19 +++++++++++++----- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------- t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 67 insertions(+), 25 deletions(-)
Documentation/git-name-rev.txt +28 -1
@@ -43,10 +43,37 @@ OPTIONS List all commits reachable from all refs --stdin:: + This option is deprecated in favor of 'git name-rev --annotate-stdin'. + They are functionally equivalent. + +--annotate-stdin:: 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. Intended for the scripter's use. + altogether. + + For example: ++ +---------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 +(master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name is master, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad +---------- --name-only:: Instead of printing both the SHA-1 and the name, print only
builtin/name-rev.c +14 -5
@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_stdin =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),
@@ -539,6 +539,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)OPT_GROUP ("" ),OPT_BOOL (0 ,"all" , &all,N_ ("list all commits reachable from all refs" )),OPT_BOOL (0 ,"stdin" , &transform_stdin,N_ ("read from stdin" )), +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, N_("show abbreviated commit object as fallback")),
@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-stdin instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_stdin =1 ; + } + +if (all + annotate_stdin + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_stdin) cutoff =0 ;for (; argc; argc--, argv++) {
@@ -613,8 +622,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { -char buffer[2048 ]; +if (annotate_stdin) { +struct strbuf sb =STRBUF_INIT ;while (!feof (stdin)) {char *p =fgets (buffer,sizeof (buffer), stdin);
t/t3412-rebase-root.sh +1 -1
@@ -11,7 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME log_with_names () { git rev-list --topo-order --parents --pretty="tformat:%s" HEAD | - git name-rev --stdin --name-only --refs=refs/heads/$1 + git name-rev --annotate-stdin --name-only --refs=refs/heads/$1 }
t/t4202-log.sh +1 -1
@@ -659,7 +659,7 @@ EOF test_expect_success 'log --graph with full output' ' git log --graph --date-order --pretty=short | - git name-rev --name-only --stdin | + git name-rev --name-only --annotate-stdin | sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual && test_cmp expect actual '
t/t6007-rev-list-cherry-pick-file.sh +13 -13
@@ -58,7 +58,7 @@ EOF test_expect_success '--left-right' ' git rev-list --left-right B...C > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -78,14 +78,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty' ' git rev-list --left-right --cherry-pick B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'bar does not come up empty' ' git rev-list --left-right B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -97,14 +97,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty (II)' ' git rev-list --left-right --cherry-pick F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev multiple --refs combine inclusive' ' git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \ <actual >actual.named && test_cmp expect actual.named '
@@ -116,7 +116,7 @@ EOF test_expect_success 'name-rev --refs excludes non-matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" \ <actual >actual.named && test_cmp expect actual.named '
@@ -128,14 +128,14 @@ EOF test_expect_success 'name-rev --exclude excludes matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \ <actual >actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev --no-refs clears the refs list' ' git rev-list --left-right --cherry-pick F...E -- bar >expect && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ <expect >actual && test_cmp expect actual '
@@ -149,7 +149,7 @@ EOF test_expect_success '--cherry-mark' ' git rev-list --cherry-mark F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -163,7 +163,7 @@ EOF test_expect_success '--cherry-mark --left-right' ' git rev-list --cherry-mark --left-right F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -174,14 +174,14 @@ EOF test_expect_success '--cherry-pick --right-only' ' git rev-list --cherry-pick --right-only F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success '--cherry-pick --left-only' ' git rev-list --cherry-pick --left-only E...F -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -193,7 +193,7 @@ EOF test_expect_success '--cherry' ' git rev-list --cherry F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
t/t6012-rev-list-simplify.sh +1 -1
@@ -12,7 +12,7 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" + git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" } #
t/t6111-rev-list-treesame.sh +2 -1
@@ -23,7 +23,8 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" + git name-rev --tags --annotate-stdin | \ + sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" } test_expect_success setup '
t/t6120-describe.sh +7 -2
@@ -270,7 +270,7 @@ test_expect_success 'name-rev --all' ' test_cmp expect actual ' -test_expect_success 'name-rev --stdin' ' +test_expect_success 'name-rev --annotate-stdin' ' >expect.unsorted && for rev in $(git rev-list --all) do
@@ -278,11 +278,16 @@ test_expect_success 'name-rev --stdin' ' echo "$rev ($name)" >>expect.unsorted || return 1 done && sort <expect.unsorted >expect && - git rev-list --all | git name-rev --stdin >actual.unsorted && + git rev-list --all | git name-rev --annotate-stdin >actual.unsorted && sort <actual.unsorted >actual && test_cmp expect actual ' +test_expect_success 'name-rev --stdin deprecated' " + git rev-list --all | git name-rev --stdin 2>actual && + grep -E 'warning: --stdin is deprecated' actual +" + test_expect_success 'describe --contains with the exact tags' ' echo "A^0" >expect && tag_object=$(git rev-parse refs/tags/A) &&
Signature · 1 line
gitgitgadget
PATCH v3 2/2 name-rev.c: use strbuf_getline instead of limited size buffer
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
Using a buffer limited to 2048 is unnecessarily limiting. Switch to using a string buffer to read in stdin for annotation.
--- builtin/name-rev.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
builtin/name-rev.c +3 -5
@@ -625,12 +625,10 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)if (annotate_stdin) {struct strbuf sb =STRBUF_INIT ; -while (!feof (stdin)) { -char *p =fgets (buffer,sizeof (buffer), stdin); -if (!p) -break ; -name_rev_line (p, &data); +while (strbuf_getwholeline (&sb, stdin,'\n' ) !=EOF ) { +name_rev_line (sb.buf, &data); } +strbuf_release (&sb); }else if (all) {int i, max;
Signature · 1 line
gitgitgadget
Re: [PATCH v3 2/2] name-rev.c: use strbuf_getline instead of limited size buffer
Quoted 15 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Using a buffer limited to 2048 is unnecessarily limiting. Switch to using a string buffer to read in stdin for annotation.
Signed-off-by "John Cai" <[email protected]>--- builtin/name-rev.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)builtin/name-rev.c +0
-0
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 628
Quoted 9 lines
builtin/name-rev.c +1
-5@@ -625,12 +625,10 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)if (annotate_stdin) {struct strbuf sb =STRBUF_INIT ; -while (!feof (stdin)) { -char *p =fgets (buffer,sizeof (buffer), stdin); -if (!p) -break ; -name_rev_line (p, &data); +while (strbuf_getwholeline (&sb, stdin,'\n' ) !=EOF ) {
As we are reading text, I wonder if it makes more sense to just use strbuf_getline() that is meant to use the definition of "line" that honors the platform convention (i.e. on Windows, '\r\n' is taken as the EOL marker).
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 633
Quoted 5 lines
builtin/name-rev.c +2
-0+name_rev_line (sb.buf, &data); } +strbuf_release (&sb); }else if (all) {int i, max;
Other than that, looking good.
Thanks.
Documentation/git-name-rev.txt OPTIONS · line 56
Quoted 9 lines · John Cai via GitGitGadget
+--annotate-stdin::
Documentation/git-name-rev.txt +4
-1Transform 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. Intended for the scripter's use. + altogether. + + For example: ++
I wonder if you also need the "There is no paragraph break, concluding the bulleted item" marker that is a sole plus sign on a line before the "For example:" thing.
Other than that, looking good.
Thanks.
Quoted 3 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Introduce a --annotate-text that is functionally equivalent of --stdin.
This line and the title need to be updated to use "--annotate-stdin".
Quoted 29 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Introduce a --annotate-text that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-text.
This change adds a warning to --stdin warning that it will be removed in the future.
Signed-off-by "John Cai" <[email protected]>--- Documentation/git-name-rev.txt | 29 +++++++++++++++++++++++++++- builtin/name-rev.c | 19 +++++++++++++----- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------- t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 67 insertions(+), 25 deletions(-)Documentation/git-name-rev.txt +0
-0@@ -43,10 +43,37 @@ OPTIONS List all commits reachable from all refs
Documentation/git-name-rev.txt OPTIONS · line 53
Quoted 10 lines
Documentation/git-name-rev.txt +5
-1--stdin:: + This option is deprecated in favor of 'git name-rev --annotate-stdin'. + They are functionally equivalent. + +--annotate-stdin:: 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. Intended for the scripter's use. + altogether.
Is there a preferred order for the old/new variant documentation?
It struck me that the docs should offer the new (now 'correct') variant first.
I didn't see anything about deprecation in the CodingGuidlines or SubmittingPatches (only a quick search though)
--
Philip
Quoted 281 lines
Documentation/git-name-rev.txt +23
-0+ + For example: ++ +---------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 +(master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name is master, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad +---------- --name-only:: Instead of printing both the SHA-1 and the name, print onlybuiltin/name-rev.c +14
-5@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_stdin =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),@@ -539,6 +539,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)OPT_GROUP ("" ),OPT_BOOL (0 ,"all" , &all,N_ ("list all commits reachable from all refs" )),OPT_BOOL (0 ,"stdin" , &transform_stdin,N_ ("read from stdin" )), +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, N_("show abbreviated commit object as fallback")),@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-stdin instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_stdin =1 ; + } + +if (all + annotate_stdin + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_stdin) cutoff =0 ;for (; argc; argc--, argv++) {@@ -613,8 +622,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { -char buffer[2048 ]; +if (annotate_stdin) { +struct strbuf sb =STRBUF_INIT ;while (!feof (stdin)) {char *p =fgets (buffer,sizeof (buffer), stdin);t/t3412-rebase-root.sh +1
-1@@ -11,7 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME log_with_names () { git rev-list --topo-order --parents --pretty="tformat:%s" HEAD | - git name-rev --stdin --name-only --refs=refs/heads/$1 + git name-rev --annotate-stdin --name-only --refs=refs/heads/$1 }t/t4202-log.sh +1
-1@@ -659,7 +659,7 @@ EOF test_expect_success 'log --graph with full output' ' git log --graph --date-order --pretty=short | - git name-rev --name-only --stdin | + git name-rev --name-only --annotate-stdin | sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual && test_cmp expect actual 't/t6007-rev-list-cherry-pick-file.sh +13
-13@@ -58,7 +58,7 @@ EOF test_expect_success '--left-right' ' git rev-list --left-right B...C > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -78,14 +78,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty' ' git rev-list --left-right --cherry-pick B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'bar does not come up empty' ' git rev-list --left-right B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -97,14 +97,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty (II)' ' git rev-list --left-right --cherry-pick F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev multiple --refs combine inclusive' ' git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \ <actual >actual.named && test_cmp expect actual.named '@@ -116,7 +116,7 @@ EOF test_expect_success 'name-rev --refs excludes non-matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" \ <actual >actual.named && test_cmp expect actual.named '@@ -128,14 +128,14 @@ EOF test_expect_success 'name-rev --exclude excludes matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \ <actual >actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev --no-refs clears the refs list' ' git rev-list --left-right --cherry-pick F...E -- bar >expect && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ <expect >actual && test_cmp expect actual '@@ -149,7 +149,7 @@ EOF test_expect_success '--cherry-mark' ' git rev-list --cherry-mark F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -163,7 +163,7 @@ EOF test_expect_success '--cherry-mark --left-right' ' git rev-list --cherry-mark --left-right F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -174,14 +174,14 @@ EOF test_expect_success '--cherry-pick --right-only' ' git rev-list --cherry-pick --right-only F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success '--cherry-pick --left-only' ' git rev-list --cherry-pick --left-only E...F -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -193,7 +193,7 @@ EOF test_expect_success '--cherry' ' git rev-list --cherry F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named 't/t6012-rev-list-simplify.sh +1
-1@@ -12,7 +12,7 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" + git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" } #t/t6111-rev-list-treesame.sh +2
-1@@ -23,7 +23,8 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" + git name-rev --tags --annotate-stdin | \ + sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" } test_expect_success setup 't/t6120-describe.sh +7
-2@@ -270,7 +270,7 @@ test_expect_success 'name-rev --all' ' test_cmp expect actual ' -test_expect_success 'name-rev --stdin' ' +test_expect_success 'name-rev --annotate-stdin' ' >expect.unsorted && for rev in $(git rev-list --all) do@@ -278,11 +278,16 @@ test_expect_success 'name-rev --stdin' ' echo "$rev ($name)" >>expect.unsorted || return 1 done && sort <expect.unsorted >expect && - git rev-list --all | git name-rev --stdin >actual.unsorted && + git rev-list --all | git name-rev --annotate-stdin >actual.unsorted && sort <actual.unsorted >actual && test_cmp expect actual ' +test_expect_success 'name-rev --stdin deprecated' " + git rev-list --all | git name-rev --stdin 2>actual && + grep -E 'warning: --stdin is deprecated' actual +" + test_expect_success 'describe --contains with the exact tags' ' echo "A^0" >expect && tag_object=$(git rev-parse refs/tags/A) &&
v4 cover name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetIntroduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, the goal is to rename it to --annotate-stdin.
This is the first step in a process of eventually fully deprecating --stdin. This change also adds a warning to --stdin warning that it will be removed in the future.
See https://lore.kernel.org/git/[email protected]/ for discussion.
changes since v3:
* use strbuf_getline instead of strbuf_getwholeline (based on Junio's feedback)
* fixed commit message s/annotate-text/annotate-stdin (based on Junio's feedback)
* since strbuf_getline does not keep the trailing terminator, add back '\n' with strbuf_addchr
* reordered documentation blocks based on (Philip Oakley's feedback)
* fixed doc typos in example block
John Cai (2):
name-rev: deprecate --stdin in favor of --annotate-stdin name-rev.c: use strbuf_getline instead of limited size buffer
Documentation/git-name-rev.txt | 30 ++++++++++++++++++++++++++-- builtin/name-rev.c | 28 ++++++++++++++++---------- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------ t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 71 insertions(+), 31 deletions(-)
base-commit: c8b2ade48c204690119936ada89cd938c476c5c2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1171%2Fjohn-cai%2Fjc%2Fdeprecate-name-rev-stdin-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1171/john-cai/jc/deprecate-name-rev-stdin-v4
Pull-Request: https://github.com/git/git/pull/1171
Range-diff vs v3:
1: 55ec2a5fa3e ! 1: 4e9200922a4 name-rev: deprecate --stdin in favor of --annotate-text
@@ Metadata
Author: John Cai <[email protected]>
## Commit message ##
- name-rev: deprecate --stdin in favor of --annotate-text
+ name-rev: deprecate --stdin in favor of --annotate-stdin
- Introduce a --annotate-text that is functionally equivalent of --stdin.
+ Introduce a --annotate-stdin that is functionally equivalent of --stdin.
--stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can
- be a confusing and misleading name, rename it to --annotate-text.
+ be a confusing and misleading name, rename it to --annotate-stdin.
This change adds a warning to --stdin warning that it will be removed in the future.
@@ Commit message
## Documentation/git-name-rev.txt ##
@@ Documentation/git-name-rev.txt: OPTIONS
+ --all::
List all commits reachable from all refs
- --stdin::
-+ This option is deprecated in favor of 'git name-rev --annotate-stdin'.
-+ They are functionally equivalent.
-+
+---stdin::
+--annotate-stdin::
Transform stdin by substituting all the 40-character SHA-1
hexes (say $hex) with "$hex ($rev_name)". When used with
@@ Documentation/git-name-rev.txt: OPTIONS
- altogether. Intended for the scripter's use.
+ altogether.
+
-+ For example:
-++
++For example:
++
+----------
+$ cat sample.txt
+
@@ Documentation/git-name-rev.txt: OPTIONS
+$ git name-rev --annotate-stdin <sample.txt
+
+An abbreviated revision 2ae0a9cb82 will not be substituted.
-+The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 -+(master),
++The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master),
+while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
+
+$ git name-rev --name-only --annotate-stdin <sample.txt
+
+An abbreviated revision 2ae0a9cb82 will not be substituted.
-+The full name is master,
++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
2: e4bd09ccf75 ! 2: 18f77ab9dde name-rev.c: use strbuf_getline instead of limited size buffer
@@ builtin/name-rev.c: int cmd_name_rev(int argc, const char **argv, const char *pr
- if (!p)
- break;
- name_rev_line(p, &data);
-+ while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
++ while (strbuf_getline(&sb, stdin) != EOF) {
++ strbuf_addch(&sb, '\n');
+ name_rev_line(sb.buf, &data);
}
+ strbuf_release(&sb);Signature · 1 line
gitgitgadget
PATCH v4 1/2 name-rev: deprecate --stdin in favor of --annotate-stdin
From: John Cai <[email protected]>
Introduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-stdin.
This change adds a warning to --stdin warning that it will be removed in the future.
--- Documentation/git-name-rev.txt | 30 ++++++++++++++++++++++++++-- builtin/name-rev.c | 19 +++++++++++++----- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------ t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 67 insertions(+), 26 deletions(-)
Documentation/git-name-rev.txt +28 -2
@@ -42,11 +42,37 @@ OPTIONS --all:: List all commits reachable from all refs ---stdin:: +--annotate-stdin:: 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. Intended for the scripter's use. + altogether. + +For example: + +---------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +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
builtin/name-rev.c +14 -5
@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_stdin =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),
@@ -539,6 +539,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)OPT_GROUP ("" ),OPT_BOOL (0 ,"all" , &all,N_ ("list all commits reachable from all refs" )),OPT_BOOL (0 ,"stdin" , &transform_stdin,N_ ("read from stdin" )), +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, N_("show abbreviated commit object as fallback")),
@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-stdin instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_stdin =1 ; + } + +if (all + annotate_stdin + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_stdin) cutoff =0 ;for (; argc; argc--, argv++) {
@@ -613,8 +622,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { -char buffer[2048 ]; +if (annotate_stdin) { +struct strbuf sb =STRBUF_INIT ;while (!feof (stdin)) {char *p =fgets (buffer,sizeof (buffer), stdin);
t/t3412-rebase-root.sh +1 -1
@@ -11,7 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME log_with_names () { git rev-list --topo-order --parents --pretty="tformat:%s" HEAD | - git name-rev --stdin --name-only --refs=refs/heads/$1 + git name-rev --annotate-stdin --name-only --refs=refs/heads/$1 }
t/t4202-log.sh +1 -1
@@ -659,7 +659,7 @@ EOF test_expect_success 'log --graph with full output' ' git log --graph --date-order --pretty=short | - git name-rev --name-only --stdin | + git name-rev --name-only --annotate-stdin | sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual && test_cmp expect actual '
t/t6007-rev-list-cherry-pick-file.sh +13 -13
@@ -58,7 +58,7 @@ EOF test_expect_success '--left-right' ' git rev-list --left-right B...C > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -78,14 +78,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty' ' git rev-list --left-right --cherry-pick B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'bar does not come up empty' ' git rev-list --left-right B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -97,14 +97,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty (II)' ' git rev-list --left-right --cherry-pick F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev multiple --refs combine inclusive' ' git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \ <actual >actual.named && test_cmp expect actual.named '
@@ -116,7 +116,7 @@ EOF test_expect_success 'name-rev --refs excludes non-matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" \ <actual >actual.named && test_cmp expect actual.named '
@@ -128,14 +128,14 @@ EOF test_expect_success 'name-rev --exclude excludes matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \ <actual >actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev --no-refs clears the refs list' ' git rev-list --left-right --cherry-pick F...E -- bar >expect && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ <expect >actual && test_cmp expect actual '
@@ -149,7 +149,7 @@ EOF test_expect_success '--cherry-mark' ' git rev-list --cherry-mark F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -163,7 +163,7 @@ EOF test_expect_success '--cherry-mark --left-right' ' git rev-list --cherry-mark --left-right F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -174,14 +174,14 @@ EOF test_expect_success '--cherry-pick --right-only' ' git rev-list --cherry-pick --right-only F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success '--cherry-pick --left-only' ' git rev-list --cherry-pick --left-only E...F -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -193,7 +193,7 @@ EOF test_expect_success '--cherry' ' git rev-list --cherry F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
t/t6012-rev-list-simplify.sh +1 -1
@@ -12,7 +12,7 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" + git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" } #
t/t6111-rev-list-treesame.sh +2 -1
@@ -23,7 +23,8 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" + git name-rev --tags --annotate-stdin | \ + sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" } test_expect_success setup '
t/t6120-describe.sh +7 -2
@@ -270,7 +270,7 @@ test_expect_success 'name-rev --all' ' test_cmp expect actual ' -test_expect_success 'name-rev --stdin' ' +test_expect_success 'name-rev --annotate-stdin' ' >expect.unsorted && for rev in $(git rev-list --all) do
@@ -278,11 +278,16 @@ test_expect_success 'name-rev --stdin' ' echo "$rev ($name)" >>expect.unsorted || return 1 done && sort <expect.unsorted >expect && - git rev-list --all | git name-rev --stdin >actual.unsorted && + git rev-list --all | git name-rev --annotate-stdin >actual.unsorted && sort <actual.unsorted >actual && test_cmp expect actual ' +test_expect_success 'name-rev --stdin deprecated' " + git rev-list --all | git name-rev --stdin 2>actual && + grep -E 'warning: --stdin is deprecated' actual +" + test_expect_success 'describe --contains with the exact tags' ' echo "A^0" >expect && tag_object=$(git rev-parse refs/tags/A) &&
Signature · 1 line
gitgitgadget
PATCH v4 2/2 name-rev.c: use strbuf_getline instead of limited size buffer
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
Using a buffer limited to 2048 is unnecessarily limiting. Switch to using a string buffer to read in stdin for annotation.
--- builtin/name-rev.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
builtin/name-rev.c +4 -5
@@ -625,12 +625,11 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)if (annotate_stdin) {struct strbuf sb =STRBUF_INIT ; -while (!feof (stdin)) { -char *p =fgets (buffer,sizeof (buffer), stdin); -if (!p) -break ; -name_rev_line (p, &data); +while (strbuf_getline (&sb, stdin) !=EOF ) { +strbuf_addch (&sb,'\n' ); +name_rev_line (sb.buf, &data); } +strbuf_release (&sb); }else if (all) {int i, max;
Signature · 1 line
gitgitgadget
Re: [PATCH v4 2/2] name-rev.c: use strbuf_getline instead of limited size buffer
Quoted 19 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Using a buffer limited to 2048 is unnecessarily limiting. Switch to using a string buffer to read in stdin for annotation.
Signed-off-by "John Cai" <[email protected]>---
builtin/name-rev.c| 9 ++++-----1 file changed, 4 insertions(+), 5 deletions(-)builtin/name-rev.c +0
-0
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 629
Quoted 10 lines
builtin/name-rev.c +2
-5@@ -625,12 +625,11 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)if (annotate_stdin) {struct strbuf sb =STRBUF_INIT ; -while (!feof (stdin)) { -char *p =fgets (buffer,sizeof (buffer), stdin); -if (!p) -break ; -name_rev_line (p, &data); +while (strbuf_getline (&sb, stdin) !=EOF ) { +strbuf_addch (&sb,'\n' );
I agree strbuf_getline is better since it handles EOF across platforms. However, one trouble I ran into is that it does not retain the line terminator, so I had to add it back in this fashion. It looks a little ugly, but let me know if you think this is preferable to using strbuf_getwholeline.
Quoted 8 lines
builtin/name-rev.c +2
-0+name_rev_line (sb.buf, &data); } +strbuf_release (&sb); }else if (all) {int i, max;Signature · 1 line
gitgitgadget
Quoted 4 lines · Philip Oakley
Quoted 4 lines
--stdin::
Documentation/git-name-rev.txt +3
-0+ This option is deprecated in favor of 'git name-rev --annotate-stdin'. + They are functionally equivalent. +
Quoted 8 lines
Documentation/git-name-rev.txt OPTIONS · line 53
Quoted 6 lines
+--annotate-stdin::
Documentation/git-name-rev.txt +1
-1Transform 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. Intended for the scripter's use. + altogether.Is there a preferred order for the old/new variant documentation?
If the surrounding list has a natural order of its own, e.g. the list is sorted alphabetically to help people locate an item by name, it is OK to have old and new in that order and older one may appear before the newer one.
But otherwise it may make sense to show the newer one first with the mention of the older one as if it were a side note, especially when they can appear next to each other like the one we are looking at, I would think.
I do not think we have written preference. It probably would help to have a short paragraph in the coding guidelines document.
Thanks.
Quoted 13 lines · Philip Oakley
Documentation/git-name-rev.txt OPTIONS · line 53
Quoted 11 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
--stdin::Documentation/git-name-rev.txt +5
-1+ This option is deprecated in favor of 'git name-rev --annotate-stdin'. + They are functionally equivalent. + +--annotate-stdin:: 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. Intended for the scripter's use. + altogether.
Quoted 7 lines
Is there a preferred order for the old/new variant documentation?
It struck me that the docs should offer the new (now 'correct') variant first.
I didn't see anything about deprecation in the CodingGuidlines or SubmittingPatches (only a quick search though)
In other cases, we've demoted the deprecated option to a tiny blurb at the end of the description of the option which replaces it so that the deprecated option is no longer given a place of prominence but can still be found with a search in case someone runs across it somewhere and wants to learn about it.
--annotate-stdin::
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. --stdin is a deprecated synonym.
Re: [PATCH v4 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetReview
Quoted 6 lines · John Cai via GitGitGadget
(quoted diff) +6
-0+ altogether. + +For example: + +---------- +$ cat sample.txt
Because this example is a part of description of --annotate-stdin, not an example after a list that ends with --annotate-stdin, you would want to tweak the above a bit.
Documentation/git-name-rev.txt +2 -2
@@ -47,9 +47,9 @@ OPTIONS hexes (say $hex) with "$hex ($rev_name)". When used with --name-only, substitute with "$rev_name", omitting $hex altogether. - ++ For example: - ++ ---------- $ cat sample.txt
v5 cover name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetIntroduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, the goal is to rename it to --annotate-stdin.
This is the first step in a process of eventually fully deprecating --stdin. This change also adds a warning to --stdin warning that it will be removed in the future.
See https://lore.kernel.org/git/[email protected]/ for discussion.
changes since v4:
* fixed documentation example indentation
changes since v3:
* use strbuf_getline instead of strbuf_getwholeline (based on Junio's feedback)
* fixed commit message s/annotate-text/annotate-stdin (based on Junio's feedback)
* since strbuf_getline does not keep the trailing terminator, add back '\n' with strbuf_addchr
* reordered documentation blocks based on (Philip Oakley's feedback)
* fixed doc typos in example block
John Cai (2):
name-rev: deprecate --stdin in favor of --annotate-stdin name-rev.c: use strbuf_getline instead of limited size buffer
Documentation/git-name-rev.txt | 30 ++++++++++++++++++++++++++-- builtin/name-rev.c | 28 ++++++++++++++++---------- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------ t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 71 insertions(+), 31 deletions(-)
base-commit: c8b2ade48c204690119936ada89cd938c476c5c2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1171%2Fjohn-cai%2Fjc%2Fdeprecate-name-rev-stdin-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1171/john-cai/jc/deprecate-name-rev-stdin-v5
Pull-Request: https://github.com/git/git/pull/1171
Range-diff vs v4:
1: 4e9200922a4 ! 1: 7c5fb10d87c name-rev: deprecate --stdin in favor of --annotate-stdin
@@ Documentation/git-name-rev.txt: OPTIONS
--name-only, substitute with "$rev_name", omitting $hex
- altogether. Intended for the scripter's use.
+ altogether.
-+
+++
+For example:
+++
++--
++ $ cat sample.txt
+
-+----------
-+$ cat sample.txt
-+
-+An abbreviated revision 2ae0a9cb82 will not be substituted.
-+The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907,
-+while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
++ An abbreviated revision 2ae0a9cb82 will not be substituted.
++ The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907,
++ while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
+
-+$ git name-rev --annotate-stdin <sample.txt
++ $ git name-rev --annotate-stdin <sample.txt
+
-+An abbreviated revision 2ae0a9cb82 will not be substituted.
-+The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master),
-+while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
++ An abbreviated revision 2ae0a9cb82 will not be substituted.
++ The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master),
++ while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
+
-+$ git name-rev --name-only --annotate-stdin <sample.txt
++ $ git name-rev --name-only --annotate-stdin <sample.txt
+
-+An abbreviated revision 2ae0a9cb82 will not be substituted.
-+The full name after substitution is master,
-+while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
-+----------
++ An abbreviated revision 2ae0a9cb82 will not be substituted.
++ 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'. 2: 18f77ab9dde = 2: 2acd70f36e7 name-rev.c: use strbuf_getline instead of limited size buffer
Signature · 1 line
gitgitgadget
PATCH v5 2/2 name-rev.c: use strbuf_getline instead of limited size buffer
From: John Cai <[email protected]>
Using a buffer limited to 2048 is unnecessarily limiting. Switch to using a string buffer to read in stdin for annotation.
--- builtin/name-rev.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
builtin/name-rev.c +4 -5
@@ -625,12 +625,11 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)if (annotate_stdin) {struct strbuf sb =STRBUF_INIT ; -while (!feof (stdin)) { -char *p =fgets (buffer,sizeof (buffer), stdin); -if (!p) -break ; -name_rev_line (p, &data); +while (strbuf_getline (&sb, stdin) !=EOF ) { +strbuf_addch (&sb,'\n' ); +name_rev_line (sb.buf, &data); } +strbuf_release (&sb); }else if (all) {int i, max;
Signature · 1 line
gitgitgadget
PATCH v5 1/2 name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
Introduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-stdin.
This change adds a warning to --stdin warning that it will be removed in the future.
--- Documentation/git-name-rev.txt | 30 ++++++++++++++++++++++++++-- builtin/name-rev.c | 19 +++++++++++++----- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------ t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 67 insertions(+), 26 deletions(-)
Documentation/git-name-rev.txt +28 -2
@@ -42,11 +42,37 @@ OPTIONS --all:: List all commits reachable from all refs ---stdin:: +--annotate-stdin:: 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. Intended for the scripter's use. + altogether. ++ +For example: ++ +-- + $ cat sample.txt + + An abbreviated revision 2ae0a9cb82 will not be substituted. + The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, + while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + + $ git name-rev --annotate-stdin <sample.txt + + An abbreviated revision 2ae0a9cb82 will not be substituted. + The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master), + while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + + $ git name-rev --name-only --annotate-stdin <sample.txt + + An abbreviated revision 2ae0a9cb82 will not be substituted. + 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
builtin/name-rev.c +14 -5
@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_stdin =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),
@@ -539,6 +539,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)OPT_GROUP ("" ),OPT_BOOL (0 ,"all" , &all,N_ ("list all commits reachable from all refs" )),OPT_BOOL (0 ,"stdin" , &transform_stdin,N_ ("read from stdin" )), +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, N_("show abbreviated commit object as fallback")),
@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-stdin instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_stdin =1 ; + } + +if (all + annotate_stdin + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_stdin) cutoff =0 ;for (; argc; argc--, argv++) {
@@ -613,8 +622,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { -char buffer[2048 ]; +if (annotate_stdin) { +struct strbuf sb =STRBUF_INIT ;while (!feof (stdin)) {char *p =fgets (buffer,sizeof (buffer), stdin);
t/t3412-rebase-root.sh +1 -1
@@ -11,7 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME log_with_names () { git rev-list --topo-order --parents --pretty="tformat:%s" HEAD | - git name-rev --stdin --name-only --refs=refs/heads/$1 + git name-rev --annotate-stdin --name-only --refs=refs/heads/$1 }
t/t4202-log.sh +1 -1
@@ -659,7 +659,7 @@ EOF test_expect_success 'log --graph with full output' ' git log --graph --date-order --pretty=short | - git name-rev --name-only --stdin | + git name-rev --name-only --annotate-stdin | sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual && test_cmp expect actual '
t/t6007-rev-list-cherry-pick-file.sh +13 -13
@@ -58,7 +58,7 @@ EOF test_expect_success '--left-right' ' git rev-list --left-right B...C > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -78,14 +78,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty' ' git rev-list --left-right --cherry-pick B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'bar does not come up empty' ' git rev-list --left-right B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -97,14 +97,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty (II)' ' git rev-list --left-right --cherry-pick F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev multiple --refs combine inclusive' ' git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \ <actual >actual.named && test_cmp expect actual.named '
@@ -116,7 +116,7 @@ EOF test_expect_success 'name-rev --refs excludes non-matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" \ <actual >actual.named && test_cmp expect actual.named '
@@ -128,14 +128,14 @@ EOF test_expect_success 'name-rev --exclude excludes matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \ <actual >actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev --no-refs clears the refs list' ' git rev-list --left-right --cherry-pick F...E -- bar >expect && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ <expect >actual && test_cmp expect actual '
@@ -149,7 +149,7 @@ EOF test_expect_success '--cherry-mark' ' git rev-list --cherry-mark F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -163,7 +163,7 @@ EOF test_expect_success '--cherry-mark --left-right' ' git rev-list --cherry-mark --left-right F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -174,14 +174,14 @@ EOF test_expect_success '--cherry-pick --right-only' ' git rev-list --cherry-pick --right-only F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success '--cherry-pick --left-only' ' git rev-list --cherry-pick --left-only E...F -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -193,7 +193,7 @@ EOF test_expect_success '--cherry' ' git rev-list --cherry F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
t/t6012-rev-list-simplify.sh +1 -1
@@ -12,7 +12,7 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" + git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" } #
t/t6111-rev-list-treesame.sh +2 -1
@@ -23,7 +23,8 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" + git name-rev --tags --annotate-stdin | \ + sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" } test_expect_success setup '
t/t6120-describe.sh +7 -2
@@ -270,7 +270,7 @@ test_expect_success 'name-rev --all' ' test_cmp expect actual ' -test_expect_success 'name-rev --stdin' ' +test_expect_success 'name-rev --annotate-stdin' ' >expect.unsorted && for rev in $(git rev-list --all) do
@@ -278,11 +278,16 @@ test_expect_success 'name-rev --stdin' ' echo "$rev ($name)" >>expect.unsorted || return 1 done && sort <expect.unsorted >expect && - git rev-list --all | git name-rev --stdin >actual.unsorted && + git rev-list --all | git name-rev --annotate-stdin >actual.unsorted && sort <actual.unsorted >actual && test_cmp expect actual ' +test_expect_success 'name-rev --stdin deprecated' " + git rev-list --all | git name-rev --stdin 2>actual && + grep -E 'warning: --stdin is deprecated' actual +" + test_expect_success 'describe --contains with the exact tags' ' echo "A^0" >expect && tag_object=$(git rev-parse refs/tags/A) &&
Signature · 1 line
gitgitgadget
Re: [PATCH v4 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetHi John
Quoted 16 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
[...]builtin/name-rev.c +1
-1@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_stdin =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 541
Quoted 3 lines
builtin/name-rev.c +0
-0@@ -539,6 +539,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)OPT_GROUP ("" ),OPT_BOOL (0 ,"all" , &all,N_ ("list all commits reachable from all refs" )), OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),
If the intention is to deprecate this option then it might be worth marking it as PARSE_OPT_HIDDEN so that it is not shown by 'git name-rev -h'. (You need to change OPT_BOOL to OPT_BOOL_F to pass the flag)
Quoted 27 lines
builtin/name-rev.c +11
-2+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, N_("show abbreviated commit object as fallback")),@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-stdin instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_stdin =1 ; + } + +if (all + annotate_stdin + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_stdin) cutoff =0 ;for (; argc; argc--, argv++) {
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 626
Quoted 7 lines
builtin/name-rev.c +2
-2@@ -613,8 +622,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { -char buffer[2048 ]; +if (annotate_stdin) { +struct strbuf sb =STRBUF_INIT ;
I think this hunk belongs in the next patch. Before posting a patch series I find it helpful to run
git rebase --keep-base -x'make -j4 git && cd t && prove -j4 <tests
I think might fail> :: --root=/dev/shm'
to check that the individual patches compile and pass the relevant tests. I've never got round to trying it but git-test[1] also lets you test all the commits in a series
Best Wishes
Phillip
Quoted 204 lines
builtin/name-rev.c +0
-0while (!feof (stdin)) {char *p =fgets (buffer,sizeof (buffer), stdin);t/t3412-rebase-root.sh +1
-1@@ -11,7 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME log_with_names () { git rev-list --topo-order --parents --pretty="tformat:%s" HEAD | - git name-rev --stdin --name-only --refs=refs/heads/$1 + git name-rev --annotate-stdin --name-only --refs=refs/heads/$1 }t/t4202-log.sh +1
-1@@ -659,7 +659,7 @@ EOF test_expect_success 'log --graph with full output' ' git log --graph --date-order --pretty=short | - git name-rev --name-only --stdin | + git name-rev --name-only --annotate-stdin | sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual && test_cmp expect actual 't/t6007-rev-list-cherry-pick-file.sh +13
-13@@ -58,7 +58,7 @@ EOF test_expect_success '--left-right' ' git rev-list --left-right B...C > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -78,14 +78,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty' ' git rev-list --left-right --cherry-pick B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'bar does not come up empty' ' git rev-list --left-right B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -97,14 +97,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty (II)' ' git rev-list --left-right --cherry-pick F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev multiple --refs combine inclusive' ' git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \ <actual >actual.named && test_cmp expect actual.named '@@ -116,7 +116,7 @@ EOF test_expect_success 'name-rev --refs excludes non-matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" \ <actual >actual.named && test_cmp expect actual.named '@@ -128,14 +128,14 @@ EOF test_expect_success 'name-rev --exclude excludes matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \ <actual >actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev --no-refs clears the refs list' ' git rev-list --left-right --cherry-pick F...E -- bar >expect && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ <expect >actual && test_cmp expect actual '@@ -149,7 +149,7 @@ EOF test_expect_success '--cherry-mark' ' git rev-list --cherry-mark F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -163,7 +163,7 @@ EOF test_expect_success '--cherry-mark --left-right' ' git rev-list --cherry-mark --left-right F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -174,14 +174,14 @@ EOF test_expect_success '--cherry-pick --right-only' ' git rev-list --cherry-pick --right-only F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success '--cherry-pick --left-only' ' git rev-list --cherry-pick --left-only E...F -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -193,7 +193,7 @@ EOF test_expect_success '--cherry' ' git rev-list --cherry F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named 't/t6012-rev-list-simplify.sh +1
-1@@ -12,7 +12,7 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" + git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" } #t/t6111-rev-list-treesame.sh +2
-1@@ -23,7 +23,8 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" + git name-rev --tags --annotate-stdin | \ + sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" } test_expect_success setup 't/t6120-describe.sh +7
-2@@ -270,7 +270,7 @@ test_expect_success 'name-rev --all' ' test_cmp expect actual ' -test_expect_success 'name-rev --stdin' ' +test_expect_success 'name-rev --annotate-stdin' ' >expect.unsorted && for rev in $(git rev-list --all) do@@ -278,11 +278,16 @@ test_expect_success 'name-rev --stdin' ' echo "$rev ($name)" >>expect.unsorted || return 1 done && sort <expect.unsorted >expect && - git rev-list --all | git name-rev --stdin >actual.unsorted && + git rev-list --all | git name-rev --annotate-stdin >actual.unsorted && sort <actual.unsorted >actual && test_cmp expect actual ' +test_expect_success 'name-rev --stdin deprecated' " + git rev-list --all | git name-rev --stdin 2>actual && + grep -E 'warning: --stdin is deprecated' actual +" + test_expect_success 'describe --contains with the exact tags' ' echo "A^0" >expect && tag_object=$(git rev-parse refs/tags/A) &&
Re: [PATCH v4 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
Quoted 11 lines · Phillip Wood
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 626
Quoted 7 lines
builtin/name-rev.c +2
-1@@ -613,8 +622,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); - if (transform_stdin) { -char buffer[2048 ]; +if (annotate_stdin) { +struct strbuf sb =STRBUF_INIT ;I think this hunk belongs in the next patch. Before posting a patch series I find it helpful to run
Quoted 6 lines
git rebase --keep-base -x'make -j4 git && cd t && prove -j4 <tests I think might fail> :: --root=/dev/shm'to check that the individual patches compile and pass the relevant tests. I've never got round to trying it but git-test[1] also lets you test all the commits in a series
Ahh, I saw and pointed out exactly the same mistake in an earlier round, but failed to spot it this time around. Thanks for spotting.
Re: [PATCH v4 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to Phillip WoodQuoted 9 lines · Phillip Wood
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 541
Quoted 4 lines
builtin/name-rev.c +0
-0@@ -539,6 +539,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)OPT_GROUP ("" ),OPT_BOOL (0 ,"all" , &all,N_ ("list all commits reachable from all refs" )), OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),If the intention is to deprecate this option then it might be worth marking it as
PARSE_OPT_HIDDENso that it is not shown by 'git name-rev -h'. (You need to changeOPT_BOOLtoOPT_BOOL_Fto pass the flag)
Whether we want to do HIDDEN now or not, it is probably a good idea to update the help text to
N_("deprecated synonym to --annotate-stdin")or something like that.
Re: [PATCH v5 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetQuoted 13 lines · John Cai via GitGitGadget
Documentation/git-name-rev.txt +13
-0+ altogether. ++ +For example: ++ +-- + $ cat sample.txt + + An abbreviated revision 2ae0a9cb82 will not be substituted. + The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, + while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + + $ git name-rev --annotate-stdin <sample.txt +
Documentation/git-name-rev.txt OPTIONS · line 71
Quoted 10 lines
Documentation/git-name-rev.txt +10
-0+ An abbreviated revision 2ae0a9cb82 will not be substituted. + The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master), + while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + + $ git name-rev --name-only --annotate-stdin <sample.txt + + An abbreviated revision 2ae0a9cb82 will not be substituted. + The full name after substitution is master, + while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad +--
I compared the output from the earlier ones with this one; the way the displayed sample was shown in the previous rounds looked better, at least in HTML pages. The earlier ones used "listingblock" in which the sample is contained in a <pre>formatted <code> block, but this one ends up in a series of one-<div>-per-paragraph.
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 626
Quoted 4 lines
builtin/name-rev.c +2
-2-if (transform_stdin) { -char buffer[2048 ]; +if (annotate_stdin) { +struct strbuf sb =STRBUF_INIT ;
This will break the compilation at this step.
v6 cover name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetIntroduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, the goal is to rename it to --annotate-stdin.
This is the first step in a process of eventually fully deprecating --stdin. This change also adds a warning to --stdin warning that it will be removed in the future.
See https://lore.kernel.org/git/[email protected]/ for discussion.
changes since v5:
* fixed documentation example indentation (again)
* moved initialization of strbuf from 1/2 to 2/2
changes since v4:
* fixed documentation example indentation
changes since v3:
* use strbuf_getline instead of strbuf_getwholeline (based on Junio's feedback)
* fixed commit message s/annotate-text/annotate-stdin (based on Junio's feedback)
* since strbuf_getline does not keep the trailing terminator, add back '\n' with strbuf_addchr
* reordered documentation blocks based on (Philip Oakley's feedback)
* fixed doc typos in example block
John Cai (2):
name-rev: deprecate --stdin in favor of --annotate-stdin name-rev.c: use strbuf_getline instead of limited size buffer
Documentation/git-name-rev.txt | 30 ++++++++++++++++++++++++++-- builtin/name-rev.c | 30 ++++++++++++++++++---------- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------ t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 72 insertions(+), 32 deletions(-)
base-commit: c8b2ade48c204690119936ada89cd938c476c5c2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1171%2Fjohn-cai%2Fjc%2Fdeprecate-name-rev-stdin-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1171/john-cai/jc/deprecate-name-rev-stdin-v6
Pull-Request: https://github.com/git/git/pull/1171
Range-diff vs v5:
1: 7c5fb10d87c ! 1: 3caf254a73e name-rev: deprecate --stdin in favor of --annotate-stdin
@@ Documentation/git-name-rev.txt: OPTIONS
++
+For example:
++
-+--
-+ $ cat sample.txt
++-----------
++$ cat sample.txt
+
-+ An abbreviated revision 2ae0a9cb82 will not be substituted.
-+ The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907,
-+ while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
++An abbreviated revision 2ae0a9cb82 will not be substituted.
++The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907,
++while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
+
-+ $ git name-rev --annotate-stdin <sample.txt
++$ git name-rev --annotate-stdin <sample.txt
+
-+ An abbreviated revision 2ae0a9cb82 will not be substituted.
-+ The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master),
-+ while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
++An abbreviated revision 2ae0a9cb82 will not be substituted.
++The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master),
++while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
+
-+ $ git name-rev --name-only --annotate-stdin <sample.txt
++$ git name-rev --name-only --annotate-stdin <sample.txt
+
-+ An abbreviated revision 2ae0a9cb82 will not be substituted.
-+ The full name after substitution is master,
-+ while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad
-+--
++An abbreviated revision 2ae0a9cb82 will not be substituted.
++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'.
@@ builtin/name-rev.c: static void name_rev_line(char *p, struct name_ref_data *dat
struct option opts[] = {
OPT_BOOL(0, "name-only", &data.name_only, N_("print only ref-based names (no object names)")),
@@ builtin/name-rev.c: 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_("read from stdin")),
+- OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),
++ 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,
@@ builtin/name-rev.c: int cmd_name_rev(int argc, const char **argv, const char *pr
name_tips();
- if (transform_stdin) {
-- char buffer[2048];
+ if (annotate_stdin) {
-+ struct strbuf sb = STRBUF_INIT;
+ char buffer[2048];
while (!feof(stdin)) {
- char *p = fgets(buffer, sizeof(buffer), stdin);
## t/t3412-rebase-root.sh ##
@@ t/t3412-rebase-root.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
2: 2acd70f36e7 ! 2: 32ad96530b9 name-rev.c: use strbuf_getline instead of limited size buffer
@@ Commit message
## builtin/name-rev.c ##
@@ builtin/name-rev.c: int cmd_name_rev(int argc, const char **argv, const char *prefix)
+ name_tips();
+
if (annotate_stdin) {
- struct strbuf sb = STRBUF_INIT;
+- char buffer[2048];
++ struct strbuf sb = STRBUF_INIT;
- while (!feof(stdin)) {
- char *p = fgets(buffer, sizeof(buffer), stdin);Signature · 1 line
gitgitgadget
PATCH v6 1/2 name-rev: deprecate --stdin in favor of --annotate-stdin
From: John Cai <[email protected]>
Introduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-stdin.
This change adds a warning to --stdin warning that it will be removed in the future.
--- Documentation/git-name-rev.txt | 30 ++++++++++++++++++++++++++-- builtin/name-rev.c | 19 +++++++++++++----- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------ t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 67 insertions(+), 26 deletions(-)
Documentation/git-name-rev.txt +28 -2
@@ -42,11 +42,37 @@ OPTIONS --all:: List all commits reachable from all refs ---stdin:: +--annotate-stdin:: 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. Intended for the scripter's use. + altogether. ++ +For example: ++ +----------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +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
builtin/name-rev.c +14 -5
@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_stdin =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),
@@ -538,7 +538,8 @@ 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_ ("read from stdin" )), +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, N_("show abbreviated commit object as fallback")),
@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-stdin instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_stdin =1 ; + } + +if (all + annotate_stdin + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_stdin) cutoff =0 ;for (; argc; argc--, argv++) {
@@ -613,7 +622,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { +if (annotate_stdin) {char buffer[2048 ];while (!feof (stdin)) {
t/t3412-rebase-root.sh +1 -1
@@ -11,7 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME log_with_names () { git rev-list --topo-order --parents --pretty="tformat:%s" HEAD | - git name-rev --stdin --name-only --refs=refs/heads/$1 + git name-rev --annotate-stdin --name-only --refs=refs/heads/$1 }
t/t4202-log.sh +1 -1
@@ -659,7 +659,7 @@ EOF test_expect_success 'log --graph with full output' ' git log --graph --date-order --pretty=short | - git name-rev --name-only --stdin | + git name-rev --name-only --annotate-stdin | sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual && test_cmp expect actual '
t/t6007-rev-list-cherry-pick-file.sh +13 -13
@@ -58,7 +58,7 @@ EOF test_expect_success '--left-right' ' git rev-list --left-right B...C > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -78,14 +78,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty' ' git rev-list --left-right --cherry-pick B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'bar does not come up empty' ' git rev-list --left-right B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -97,14 +97,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty (II)' ' git rev-list --left-right --cherry-pick F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev multiple --refs combine inclusive' ' git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \ <actual >actual.named && test_cmp expect actual.named '
@@ -116,7 +116,7 @@ EOF test_expect_success 'name-rev --refs excludes non-matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" \ <actual >actual.named && test_cmp expect actual.named '
@@ -128,14 +128,14 @@ EOF test_expect_success 'name-rev --exclude excludes matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \ <actual >actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev --no-refs clears the refs list' ' git rev-list --left-right --cherry-pick F...E -- bar >expect && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ <expect >actual && test_cmp expect actual '
@@ -149,7 +149,7 @@ EOF test_expect_success '--cherry-mark' ' git rev-list --cherry-mark F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -163,7 +163,7 @@ EOF test_expect_success '--cherry-mark --left-right' ' git rev-list --cherry-mark --left-right F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -174,14 +174,14 @@ EOF test_expect_success '--cherry-pick --right-only' ' git rev-list --cherry-pick --right-only F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success '--cherry-pick --left-only' ' git rev-list --cherry-pick --left-only E...F -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -193,7 +193,7 @@ EOF test_expect_success '--cherry' ' git rev-list --cherry F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
t/t6012-rev-list-simplify.sh +1 -1
@@ -12,7 +12,7 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" + git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" } #
t/t6111-rev-list-treesame.sh +2 -1
@@ -23,7 +23,8 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" + git name-rev --tags --annotate-stdin | \ + sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" } test_expect_success setup '
t/t6120-describe.sh +7 -2
@@ -270,7 +270,7 @@ test_expect_success 'name-rev --all' ' test_cmp expect actual ' -test_expect_success 'name-rev --stdin' ' +test_expect_success 'name-rev --annotate-stdin' ' >expect.unsorted && for rev in $(git rev-list --all) do
@@ -278,11 +278,16 @@ test_expect_success 'name-rev --stdin' ' echo "$rev ($name)" >>expect.unsorted || return 1 done && sort <expect.unsorted >expect && - git rev-list --all | git name-rev --stdin >actual.unsorted && + git rev-list --all | git name-rev --annotate-stdin >actual.unsorted && sort <actual.unsorted >actual && test_cmp expect actual ' +test_expect_success 'name-rev --stdin deprecated' " + git rev-list --all | git name-rev --stdin 2>actual && + grep -E 'warning: --stdin is deprecated' actual +" + test_expect_success 'describe --contains with the exact tags' ' echo "A^0" >expect && tag_object=$(git rev-parse refs/tags/A) &&
Signature · 1 line
gitgitgadget
PATCH v6 2/2 name-rev.c: use strbuf_getline instead of limited size buffer
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
Using a buffer limited to 2048 is unnecessarily limiting. Switch to using a string buffer to read in stdin for annotation.
--- builtin/name-rev.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
builtin/name-rev.c +5 -6
@@ -623,14 +623,13 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)name_tips ();if (annotate_stdin) { -char buffer[2048 ]; +struct strbuf sb =STRBUF_INIT ; -while (!feof (stdin)) { -char *p =fgets (buffer,sizeof (buffer), stdin); -if (!p) -break ; -name_rev_line (p, &data); +while (strbuf_getline (&sb, stdin) !=EOF ) { +strbuf_addch (&sb,'\n' ); +name_rev_line (sb.buf, &data); } +strbuf_release (&sb); }else if (all) {int i, max;
Signature · 1 line
gitgitgadget
Re: [PATCH v6 0/2] name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetQuoted 9 lines · John Cai via GitGitGadget
Introduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, the goal is to rename it to --annotate-stdin.
This is the first step in a process of eventually fully deprecating --stdin. This change also adds a warning to --stdin warning that it will be removed in the future.
Quoted 6 lines
See https://lore.kernel.org/git/[email protected]/ for discussion.
changes since v5:
* fixed documentation example indentation (again)
* moved initialization of strbuf from 1/2 to 2/2
This round looks almost perfect.
One I can think of that _may_ be a good idea to change from this version is to undo the HIDDEN thing, so that "git name-rev -h", without "git name-rev --help-all", will prominently say that "--stdin" is deprecated, and make it an hidden option sometime later, but that is not even "I think we should do so", but more like "if other people think it is a good idea, I would be supportive".
Thanks.
v7 cover name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetIntroduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, the goal is to rename it to --annotate-stdin.
This is the first step in a process of eventually fully deprecating --stdin. This change also adds a warning to --stdin warning that it will be removed in the future.
See https://lore.kernel.org/git/[email protected]/ for discussion.
changes since v6:
* change OPT_BOOL from hidden back to normal mode
changes since v5:
* fixed documentation example indentation (again)
* moved initialization of strbuf from 1/2 to 2/2
changes since v4:
* fixed documentation example indentation
changes since v3:
* use strbuf_getline instead of strbuf_getwholeline (based on Junio's feedback)
* fixed commit message s/annotate-text/annotate-stdin (based on Junio's feedback)
* since strbuf_getline does not keep the trailing terminator, add back '\n' with strbuf_addchr
* reordered documentation blocks based on (Philip Oakley's feedback)
* fixed doc typos in example block
John Cai (2):
name-rev: deprecate --stdin in favor of --annotate-stdin name-rev.c: use strbuf_getline instead of limited size buffer
Documentation/git-name-rev.txt | 30 ++++++++++++++++++++++++++-- builtin/name-rev.c | 30 ++++++++++++++++++---------- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------ t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 72 insertions(+), 32 deletions(-)
base-commit: c8b2ade48c204690119936ada89cd938c476c5c2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1171%2Fjohn-cai%2Fjc%2Fdeprecate-name-rev-stdin-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1171/john-cai/jc/deprecate-name-rev-stdin-v7
Pull-Request: https://github.com/git/git/pull/1171
Range-diff vs v6:
1: 3caf254a73e ! 1: 153f69ea9b6 name-rev: deprecate --stdin in favor of --annotate-stdin
@@ builtin/name-rev.c: int cmd_name_rev(int argc, const char **argv, const char *pr
OPT_GROUP(""),
OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
- OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),
-+ OPT_BOOL_F(0, "stdin", &transform_stdin, N_("deprecated: use annotate-stdin instead"), PARSE_OPT_HIDDEN),
++ OPT_BOOL(0, "stdin", &transform_stdin, N_("deprecated: use annotate-stdin instead")),
+ 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, 2: 32ad96530b9 = 2: 19e7bf96557 name-rev.c: use strbuf_getline instead of limited size buffer
Signature · 1 line
gitgitgadget
PATCH v7 1/2 name-rev: deprecate --stdin in favor of --annotate-stdin
From: John Cai <[email protected]>
Introduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-stdin.
This change adds a warning to --stdin warning that it will be removed in the future.
--- Documentation/git-name-rev.txt | 30 ++++++++++++++++++++++++++-- builtin/name-rev.c | 19 +++++++++++++----- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------ t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 67 insertions(+), 26 deletions(-)
Documentation/git-name-rev.txt +28 -2
@@ -42,11 +42,37 @@ OPTIONS --all:: List all commits reachable from all refs ---stdin:: +--annotate-stdin:: 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. Intended for the scripter's use. + altogether. ++ +For example: ++ +----------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +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
builtin/name-rev.c +14 -5
@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_stdin =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),
@@ -538,7 +538,8 @@ 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_ ("read from stdin" )), +OPT_BOOL (0 ,"stdin" , &transform_stdin,N_ ("deprecated: use annotate-stdin instead" )), +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, N_("show abbreviated commit object as fallback")),
@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-stdin instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_stdin =1 ; + } + +if (all + annotate_stdin + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_stdin) cutoff =0 ;for (; argc; argc--, argv++) {
@@ -613,7 +622,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { +if (annotate_stdin) {char buffer[2048 ];while (!feof (stdin)) {
t/t3412-rebase-root.sh +1 -1
@@ -11,7 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME log_with_names () { git rev-list --topo-order --parents --pretty="tformat:%s" HEAD | - git name-rev --stdin --name-only --refs=refs/heads/$1 + git name-rev --annotate-stdin --name-only --refs=refs/heads/$1 }
t/t4202-log.sh +1 -1
@@ -659,7 +659,7 @@ EOF test_expect_success 'log --graph with full output' ' git log --graph --date-order --pretty=short | - git name-rev --name-only --stdin | + git name-rev --name-only --annotate-stdin | sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual && test_cmp expect actual '
t/t6007-rev-list-cherry-pick-file.sh +13 -13
@@ -58,7 +58,7 @@ EOF test_expect_success '--left-right' ' git rev-list --left-right B...C > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -78,14 +78,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty' ' git rev-list --left-right --cherry-pick B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'bar does not come up empty' ' git rev-list --left-right B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -97,14 +97,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty (II)' ' git rev-list --left-right --cherry-pick F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev multiple --refs combine inclusive' ' git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \ <actual >actual.named && test_cmp expect actual.named '
@@ -116,7 +116,7 @@ EOF test_expect_success 'name-rev --refs excludes non-matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" \ <actual >actual.named && test_cmp expect actual.named '
@@ -128,14 +128,14 @@ EOF test_expect_success 'name-rev --exclude excludes matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \ <actual >actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev --no-refs clears the refs list' ' git rev-list --left-right --cherry-pick F...E -- bar >expect && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ <expect >actual && test_cmp expect actual '
@@ -149,7 +149,7 @@ EOF test_expect_success '--cherry-mark' ' git rev-list --cherry-mark F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -163,7 +163,7 @@ EOF test_expect_success '--cherry-mark --left-right' ' git rev-list --cherry-mark --left-right F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -174,14 +174,14 @@ EOF test_expect_success '--cherry-pick --right-only' ' git rev-list --cherry-pick --right-only F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success '--cherry-pick --left-only' ' git rev-list --cherry-pick --left-only E...F -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
@@ -193,7 +193,7 @@ EOF test_expect_success '--cherry' ' git rev-list --cherry F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '
t/t6012-rev-list-simplify.sh +1 -1
@@ -12,7 +12,7 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" + git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" } #
t/t6111-rev-list-treesame.sh +2 -1
@@ -23,7 +23,8 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" + git name-rev --tags --annotate-stdin | \ + sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" } test_expect_success setup '
t/t6120-describe.sh +7 -2
@@ -270,7 +270,7 @@ test_expect_success 'name-rev --all' ' test_cmp expect actual ' -test_expect_success 'name-rev --stdin' ' +test_expect_success 'name-rev --annotate-stdin' ' >expect.unsorted && for rev in $(git rev-list --all) do
@@ -278,11 +278,16 @@ test_expect_success 'name-rev --stdin' ' echo "$rev ($name)" >>expect.unsorted || return 1 done && sort <expect.unsorted >expect && - git rev-list --all | git name-rev --stdin >actual.unsorted && + git rev-list --all | git name-rev --annotate-stdin >actual.unsorted && sort <actual.unsorted >actual && test_cmp expect actual ' +test_expect_success 'name-rev --stdin deprecated' " + git rev-list --all | git name-rev --stdin 2>actual && + grep -E 'warning: --stdin is deprecated' actual +" + test_expect_success 'describe --contains with the exact tags' ' echo "A^0" >expect && tag_object=$(git rev-parse refs/tags/A) &&
Signature · 1 line
gitgitgadget
PATCH v7 2/2 name-rev.c: use strbuf_getline instead of limited size buffer
In reply to John Cai via GitGitGadgetFrom: John Cai <[email protected]>
Using a buffer limited to 2048 is unnecessarily limiting. Switch to using a string buffer to read in stdin for annotation.
--- builtin/name-rev.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
builtin/name-rev.c +5 -6
@@ -623,14 +623,13 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)name_tips ();if (annotate_stdin) { -char buffer[2048 ]; +struct strbuf sb =STRBUF_INIT ; -while (!feof (stdin)) { -char *p =fgets (buffer,sizeof (buffer), stdin); -if (!p) -break ; -name_rev_line (p, &data); +while (strbuf_getline (&sb, stdin) !=EOF ) { +strbuf_addch (&sb,'\n' ); +name_rev_line (sb.buf, &data); } +strbuf_release (&sb); }else if (all) {int i, max;
Signature · 1 line
gitgitgadget
Re: [PATCH v7 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
In reply to John Cai via GitGitGadgetHi Junio,
Quoted 80 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Introduce a --annotate-stdin that is functionally equivalent of --stdin. --stdin does not behave as --stdin in other subcommands, such as pack-objects whereby it takes one argument per line. Since --stdin can be a confusing and misleading name, rename it to --annotate-stdin.
This change adds a warning to --stdin warning that it will be removed in the future.
Signed-off-by "John Cai" <[email protected]>--- Documentation/git-name-rev.txt | 30 ++++++++++++++++++++++++++-- builtin/name-rev.c | 19 +++++++++++++----- t/t3412-rebase-root.sh | 2 +- t/t4202-log.sh | 2 +- t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++------------ t/t6012-rev-list-simplify.sh | 2 +- t/t6111-rev-list-treesame.sh | 3 ++- t/t6120-describe.sh | 9 +++++++-- 8 files changed, 67 insertions(+), 26 deletions(-)Documentation/git-name-rev.txt +28
-2@@ -42,11 +42,37 @@ OPTIONS --all:: List all commits reachable from all refs ---stdin:: +--annotate-stdin:: 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. Intended for the scripter's use. + altogether. ++ +For example: ++ +----------- +$ cat sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907, +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +The full name after substitution is 2ae0a9cb8298185a94e5998086f380a355dd8907 (master), +while its tree object is 70d105cc79e63b81cfdcb08a15297c23e60b07ad + +$ git name-rev --name-only --annotate-stdin <sample.txt + +An abbreviated revision 2ae0a9cb82 will not be substituted. +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 onlybuiltin/name-rev.c +1
-1@@ -527,7 +527,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)int cmd_name_rev (int argc,const char **argv,const char *prefix) {struct object_array revs =OBJECT_ARRAY_INIT ; -int all =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ; +int all =0 , annotate_stdin =0 , transform_stdin =0 , allow_undefined =1 , always =0 , peel_tag =0 ;struct name_ref_data data = {0 ,0 ,STRING_LIST_INIT_NODUP ,STRING_LIST_INIT_NODUP };struct option opts[] = {OPT_BOOL (0 ,"name-only" , &data.name_only,N_ ("print only ref-based names (no object names)" )),
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 543
Quoted 7 lines
builtin/name-rev.c +2
-1@@ -538,7 +538,8 @@ 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_ ("read from stdin" )), +OPT_BOOL (0 ,"stdin" , &transform_stdin,N_ ("deprecated: use annotate-stdin instead" )), +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)")),
I’ve changed this back to a non hidden bool. I believe this should be the last thing needed on this one. Let me know if anything else needs adjustment, thanks!
Quoted 236 lines
builtin/name-rev.c +11
-3OPT_BOOL (0 ,"always" , &always, N_("show abbreviated commit object as fallback")),@@ -554,11 +555,19 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)init_commit_rev_name (&rev_names);git_config (git_default_config,NULL ); argc =parse_options (argc, argv, prefix, opts, name_rev_usage,0 ); -if (all + transform_stdin + !!argc >1 ) { + +if (transform_stdin) { +warning ("--stdin is deprecated. Please use --annotate-stdin instead, " +"which is functionally equivalent.\n" +"This option will be removed in a future release." ); + annotate_stdin =1 ; + } + +if (all + annotate_stdin + !!argc >1 ) {error ("Specify either a list, or --all, not both!" );usage_with_options (name_rev_usage, opts); } -if (all || transform_stdin) +if (all || annotate_stdin) cutoff =0 ;for (; argc; argc--, argv++) {@@ -613,7 +622,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)for_each_ref (name_ref, &data);name_tips (); -if (transform_stdin) { +if (annotate_stdin) {char buffer[2048 ];while (!feof (stdin)) {t/t3412-rebase-root.sh +1
-1@@ -11,7 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME log_with_names () { git rev-list --topo-order --parents --pretty="tformat:%s" HEAD | - git name-rev --stdin --name-only --refs=refs/heads/$1 + git name-rev --annotate-stdin --name-only --refs=refs/heads/$1 }t/t4202-log.sh +1
-1@@ -659,7 +659,7 @@ EOF test_expect_success 'log --graph with full output' ' git log --graph --date-order --pretty=short | - git name-rev --name-only --stdin | + git name-rev --name-only --annotate-stdin | sed "s/Merge:.*/Merge: A B/;s/ *\$//" >actual && test_cmp expect actual 't/t6007-rev-list-cherry-pick-file.sh +13
-13@@ -58,7 +58,7 @@ EOF test_expect_success '--left-right' ' git rev-list --left-right B...C > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -78,14 +78,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty' ' git rev-list --left-right --cherry-pick B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'bar does not come up empty' ' git rev-list --left-right B...C -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -97,14 +97,14 @@ EOF test_expect_success '--cherry-pick bar does not come up empty (II)' ' git rev-list --left-right --cherry-pick F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev multiple --refs combine inclusive' ' git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" \ <actual >actual.named && test_cmp expect actual.named '@@ -116,7 +116,7 @@ EOF test_expect_success 'name-rev --refs excludes non-matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/F" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" \ <actual >actual.named && test_cmp expect actual.named '@@ -128,14 +128,14 @@ EOF test_expect_success 'name-rev --exclude excludes matched patterns' ' git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect && git rev-list --left-right --cherry-pick F...E -- bar >actual && - git name-rev --stdin --name-only --refs="*tags/*" --exclude="*E" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" --exclude="*E" \ <actual >actual.named && test_cmp expect actual.named ' test_expect_success 'name-rev --no-refs clears the refs list' ' git rev-list --left-right --cherry-pick F...E -- bar >expect && - git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ + git name-rev --annotate-stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \ <expect >actual && test_cmp expect actual '@@ -149,7 +149,7 @@ EOF test_expect_success '--cherry-mark' ' git rev-list --cherry-mark F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -163,7 +163,7 @@ EOF test_expect_success '--cherry-mark --left-right' ' git rev-list --cherry-mark --left-right F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -174,14 +174,14 @@ EOF test_expect_success '--cherry-pick --right-only' ' git rev-list --cherry-pick --right-only F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named ' test_expect_success '--cherry-pick --left-only' ' git rev-list --cherry-pick --left-only E...F -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named '@@ -193,7 +193,7 @@ EOF test_expect_success '--cherry' ' git rev-list --cherry F...E -- bar > actual && - git name-rev --stdin --name-only --refs="*tags/*" \ + git name-rev --annotate-stdin --name-only --refs="*tags/*" \ < actual > actual.named && test_cmp expect actual.named 't/t6012-rev-list-simplify.sh +1
-1@@ -12,7 +12,7 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" + git name-rev --tags --annotate-stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\)) |\1 |g" } #t/t6111-rev-list-treesame.sh +2
-1@@ -23,7 +23,8 @@ note () { } unnote () { - git name-rev --tags --stdin | sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" + git name-rev --tags --annotate-stdin | \ + sed -e "s|$OID_REGEX (tags/\([^)]*\))\([ ]\)|\1\2|g" } test_expect_success setup 't/t6120-describe.sh +7
-2@@ -270,7 +270,7 @@ test_expect_success 'name-rev --all' ' test_cmp expect actual ' -test_expect_success 'name-rev --stdin' ' +test_expect_success 'name-rev --annotate-stdin' ' >expect.unsorted && for rev in $(git rev-list --all) do@@ -278,11 +278,16 @@ test_expect_success 'name-rev --stdin' ' echo "$rev ($name)" >>expect.unsorted || return 1 done && sort <expect.unsorted >expect && - git rev-list --all | git name-rev --stdin >actual.unsorted && + git rev-list --all | git name-rev --annotate-stdin >actual.unsorted && sort <actual.unsorted >actual && test_cmp expect actual ' +test_expect_success 'name-rev --stdin deprecated' " + git rev-list --all | git name-rev --stdin 2>actual && + grep -E 'warning: --stdin is deprecated' actual +" + test_expect_success 'describe --contains with the exact tags' ' echo "A^0" >expect && tag_object=$(git rev-parse refs/tags/A) &&Signature · 1 line
gitgitgadget
Re: [PATCH v7 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
Quoted 3 lines · John Cai
Hi Junio,
...
Please trim quotes that are irrelevant for understanding what you want to say in your message to save recipient's time to find what you wrote.
Quoted 5 lines
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 543
Quoted 2 lines
builtin/name-rev.c +1
-0+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)")),I’ve changed this back to a non hidden bool. I believe this should be the last thing needed on this one. Let me know if anything else needs adjustment, thanks!
As I said, my mention of hidden vs non-hidden was not my preferrence of non-hidden over hidden, but was soliciting opinions from others, so I was a bit surprised to see a reroll to change only that thing and nothing else so soon.
But if it is your preference to leave it unhidden for now, that is fine as well. I have no strong preference over the matter either way.
I see only Dscho on the CC list, but I have this feeling that he wasn't the one who suggested to hide the old option. Whoever it was, if we can get an ack from them, that would be nicer.
Thanks for working on this.
Re: [PATCH v7 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
Cc’ing Phillip, as he was the one who made the suggestion to make it hidden
Quoted 16 lines · Junio C Hamano
Quoted 3 lines · John Cai
Hi Junio,
...
Please trim quotes that are irrelevant for understanding what you want to say in your message to save recipient's time to find what you wrote.
Quoted 5 lines
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 543
Quoted 2 lines
builtin/name-rev.c +1
-0+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)")),I’ve changed this back to a non hidden bool. I believe this should be the last thing needed on this one. Let me know if anything else needs adjustment, thanks!
Quoted 8 lines
As I said, my mention of hidden vs non-hidden was not my preferrence of non-hidden over hidden, but was soliciting opinions from others, so I was a bit surprised to see a reroll to change only that thing and nothing else so soon.
But if it is your preference to leave it unhidden for now, that is fine as well. I have no strong preference over the matter either way.
I think it makes sense in this first iteration to keep it non-hidden for visibility purposes. Maybe in the next release we can make it hidden before we move it altogether.
Quoted 6 lines
I see only Dscho on the CC list, but I have this feeling that he wasn't the one who suggested to hide the old option. Whoever it was, if we can get an ack from them, that would be nicer.
Thanks for working on this.
Re: [PATCH v7 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
Quoted 5 lines · John Cai
Quoted 5 lines
As I said, my mention of hidden vs non-hidden was not my preferrence of non-hidden over hidden, but was soliciting opinions from others, so I was a bit surprised to see a reroll to change only that thing and nothing else so soon.
Quoted 7 lines
Quoted 3 lines
But if it is your preference to leave it unhidden for now, that is fine as well. I have no strong preference over the matter either way.
I think it makes sense in this first iteration to keep it non-hidden for visibility purposes. Maybe in the next release we can make it hidden before we move it altogether.
I think that is a sensible way forward.
Re: [PATCH v7 1/2] name-rev: deprecate --stdin in favor of --annotate-stdin
Quoted 11 lines · Junio C Hamano
Quoted 9 lines · John Cai
Quoted 8 lines
As I said, my mention of hidden vs non-hidden was not my preferrence of non-hidden over hidden, but was soliciting opinions from others, so I was a bit surprised to see a reroll to change only that thing and nothing else so soon.
But if it is your preference to leave it unhidden for now, that is fine as well. I have no strong preference over the matter either way.
Quoted 5 lines
Quoted 3 lines
I think it makes sense in this first iteration to keep it non-hidden for visibility purposes. Maybe in the next release we can make it hidden before we move it altogether.
I think that is a sensible way forward.
I agree
Thanks
Phillip
Re: [PATCH v7 2/2] name-rev.c: use strbuf_getline instead of limited size buffer
In reply to John Cai via GitGitGadgetQuoted 23 lines · John Cai via GitGitGadget
From: John Cai <[email protected]>
Using a buffer limited to 2048 is unnecessarily limiting. Switch to using a string buffer to read in stdin for annotation.
Signed-off-by "John Cai" <[email protected]>--- builtin/name-rev.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)builtin/name-rev.c +1
-3@@ -623,14 +623,13 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)name_tips ();if (annotate_stdin) { -char buffer[2048 ]; +struct strbuf sb =STRBUF_INIT ; -while (!feof (stdin)) { -char *p =fgets (buffer,sizeof (buffer), stdin);
builtin/name-rev.c int cmd_name_rev(int argc, const char **argv, const char *prefix) · line 634
Quoted 10 lines
builtin/name-rev.c +4
-3-if (!p) -break ; -name_rev_line (p, &data); +while (strbuf_getline (&sb, stdin) !=EOF ) { +strbuf_addch (&sb,'\n' ); +name_rev_line (sb.buf, &data); } +strbuf_release (&sb); }else if (all) {int i, max;
Maybe there's a subtlety with \r in newlines (Windows), but isn't this doing the same thing as:
builtin/name-rev.c +1 -3
@@ -625,10 +625,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)if (annotate_stdin) {struct strbuf sb =STRBUF_INIT ; -while (strbuf_getline (&sb, stdin) !=EOF ) { -strbuf_addch (&sb,'\n' ); +while (!strbuf_getwholeline (&sb, stdin,'\n' ))name_rev_line (sb.buf, &data); - }strbuf_release (&sb); }else if (all) {int i, max;
After writing that I see this was changed on the basis of Junio's feedback in https://lore.kernel.org/git/[email protected]/ :)
FWIW I think it's fine as-is, but it also seems that name_rev_line() really doesn't care about lines per-se, but just that we don't split OIDs across "lines" (as we'll tokenize get_oid() on them). So e.g. splitting by ' ' lines (spaces) also works here, but not 'a' (as that would split a [0-9a-f].