Skip to contentSkip to conversation

rev-{list,parse}: allow -<n> as shorthand for --max-count=<n>

PATCH In review Eric Wong 2 participants 11 messages Jun 15, 2016 Kbuild

lore
Share & tools
lore

Eric Wong

author
#0 lore raw

PATCH 1/1 rev-{list,parse}: allow -<n> as shorthand for --max-count=<n>

Some versions of head(1) and tail(1) allow their line limits to be parsed this way. I find --max-count to be a commonly used option, and also similar in spirit to head/tail, so I decided to make life easier on my worn out (and lazy :) fingers with this patch.

---

 rev-list.c  |    5 +++++
 rev-parse.c |    4 ++++
 2 files changed, 9 insertions(+), 0 deletions(-)

68df4b28986a4642119373e18a63751be0e26366

rev-list.c +5 -0
index d060966..2bbd699 100644
@@ -732,6 +732,11 @@ int main(int argc, const char **argv)
 		struct commit *commit;
 		unsigned char sha1[20];
 
+		/* accept, -<digit>, like some versions of head/tail  */
+		if (*arg == '-' && isdigit(arg[1])) {
+			max_count = atoi(arg + 1);
+			continue;
+		}
 		if (!strncmp(arg, "--max-count=", 12)) {
 			max_count = atoi(arg + 12);
 			continue;
rev-parse.c +4 -0
index 0c951af..4dfc1a9 100644
@@ -52,6 +52,10 @@ static int is_rev_argument(const char *a
 	};
 	const char **p = rev_args;
 
+	/* accept, -<digit>, like some versions of head/tail  */
+	if (*arg == '-' && isdigit(arg[1]))
+		return 1;
+
 	for (;;) {
 		const char *str = *p++;
 		int len;
Signature · 1 line
1.1.4.g68df
#1 lore raw
Quoted 4 lines · Eric Wong

Some versions of head(1) and tail(1) allow their line limits to be parsed this way. I find --max-count to be a commonly used option, and also similar in spirit to head/tail, so I decided to make life easier on my worn out (and lazy :) fingers with this patch.

As an old timer, I personally am very used to "head -4", but also have been training my fingers to say "head -n 4" for the past few years, because the former is not POSIXly correct.

At the same time, I agree that --max-count *was* a mistake. We should maybe say "-n <n>" perhaps?

author
#2 lore raw
Quoted 10 lines · Junio C Hamano
Quoted 4 lines · Eric Wong

Some versions of head(1) and tail(1) allow their line limits to be parsed this way. I find --max-count to be a commonly used option, and also similar in spirit to head/tail, so I decided to make life easier on my worn out (and lazy :) fingers with this patch.

As an old timer, I personally am very used to "head -4", but also have been training my fingers to say "head -n 4" for the past few years, because the former is not POSIXly correct.

I don't agree with POSIX on this point, and I don't see why git should be bound to POSIX, especially at the UI level just because it's POSIX.

Fwiw, head/tail in coreutils distributed by Debian still supports -<n> alongside -n <n> and -n<n>.

Quoted 2 lines

At the same time, I agree that --max-count *was* a mistake. We should maybe say "-n <n>" perhaps?

Then, -n<n> (w/o the space) should be supported as well. Heck, I've been wanting GNU getopt_long() option parsing in git for a while. Not sure if I'll have time to implement it myself any time soon, though.

Signature · 1 line
Eric Wong
author
#3 lore raw

PATCH 1/1 rev-{list,parse}: allow -n<n> as shorthand for --max-count=<n>

Both -n<n> and -n <n> are supported. POSIX versions of head(1) and tail(1) allow their line limits to be parsed this way. I find --max-count to be a commonly used option, and also similar in spirit to head/tail, so I decided to make life easier on my worn out (and lazy :) fingers with this patch.

---

 rev-list.c  |   10 ++++++++++
 rev-parse.c |   19 +++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

a598ce380cfcf01b27be92bca92a3c451d3b41e3

rev-list.c +10 -0
index e00e6fc..33541cc 100644
@@ -732,6 +732,16 @@ int main(int argc, const char **argv)
 		struct commit *commit;
 		unsigned char sha1[20];
 
+		if (!strcmp(arg, "-n")) {
+			if (++i >= argc)
+				die("-n requires an argument");
+			max_count = atoi(argv[i]);
+			continue;
+		}
+		if (!strncmp(arg,"-n",2)) {
+			max_count = atoi(arg + 2);
+			continue;
+		}
 		if (!strncmp(arg, "--max-count=", 12)) {
 			max_count = atoi(arg + 12);
 			continue;
rev-parse.c +19 -0
index 7abad35..3790463 100644
@@ -21,6 +21,7 @@ static char *def = NULL;
 static int show_type = NORMAL;
 static int symbolic = 0;
 static int output_sq = 0;
+static int next_arg_is_rev = 0;
 
 static int revs_count = 0;
 
@@ -162,6 +163,24 @@ int main(int argc, char **argv)
 			show_file(arg);
 			continue;
 		}
+		if (next_arg_is_rev) {
+			if ((filter & DO_FLAGS) && (filter & DO_REVS))
+				show(arg);
+			next_arg_is_rev = 0;
+			continue;
+		}
+		if (!strcmp(arg,"-n")) {
+			next_arg_is_rev = 1;
+			if ((filter & DO_FLAGS) && (filter & DO_REVS))
+				show(arg);
+			continue;
+		}
+		if (!strncmp(arg,"-n",2)) {
+			if ((filter & DO_FLAGS) && (filter & DO_REVS))
+				show(arg);
+			continue;
+		}
+
 		if (*arg == '-') {
 			if (!strcmp(arg, "--")) {
 				as_is = 1;
Signature · 1 line
1.1.4.g3b65
author
#4 lore raw

PATCH 1/1 rev-{list,parse}: optionally allow -<n> as shorthand for --max-count=<n>

I also made this for my own private use, it's on top of the previous one I made that is POSIX-friendly. I don't if you or anyone wants it merged mainline, but I like it :)

This will only be enabled if POSIX_SHMOSIX is defined at compile-time.

Some versions of head(1) and tail(1) allow their line limits to be parsed this way. I find --max-count to be a commonly used option, and also similar in spirit to head/tail, so I decided to make life easier on my worn out (and lazy :) fingers with this patch.

---

 Makefile          |    3 +++
 git-compat-util.h |    3 +++
 rev-list.c        |    7 ++++++-
 rev-parse.c       |    4 ++++
 4 files changed, 16 insertions(+), 1 deletions(-)

3b650b00deb7e51e5c9a8bdc4cc1eaaf4fc65029

Makefile +3 -0
index 2e95353..9ef97ca 100644
@@ -367,6 +367,9 @@ endif
 ifdef NO_IPV6
 	ALL_CFLAGS += -DNO_IPV6
 endif
+ifdef POSIX_SHMOSIX
+	ALL_CFLAGS += -DPOSIX_SHMOSIX=1
+endif
 ifdef NO_SOCKADDR_STORAGE
 ifdef NO_IPV6
 	ALL_CFLAGS += -Dsockaddr_storage=sockaddr_in
git-compat-util.h +3 -0
index f982b8e..46d331d 100644
@@ -154,4 +154,7 @@ static inline int sane_case(int x, int h
 #ifndef MAXPATHLEN
 #define MAXPATHLEN 256
 #endif
+#ifndef POSIX_SHMOSIX 
+#define POSIX_SHMOSIX 0
+#endif
 #endif
rev-list.c +6 -1
index 33541cc..c85de51 100644
@@ -731,7 +731,12 @@ int main(int argc, const char **argv)
 		char *dotdot;
 		struct commit *commit;
 		unsigned char sha1[20];
-
+		
+		/* accept, -<digit>, like some versions of head/tail  */
+		if (POSIX_SHMOSIX && (*arg == '-') && isdigit(arg[1])) {
+			max_count = atoi(arg + 1);
+			continue;
+		}
 		if (!strcmp(arg, "-n")) {
 			if (++i >= argc)
 				die("-n requires an argument");
rev-parse.c +4 -0
index 3790463..d9b3fa9 100644
@@ -53,6 +53,10 @@ static int is_rev_argument(const char *a
 	};
 	const char **p = rev_args;
 
+	/* accept, -<digit>, like some versions of head/tail  */
+	if (POSIX_SHMOSIX && (*arg == '-') && isdigit(arg[1]))
+		return 1;
+
 	for (;;) {
 		const char *str = *p++;
 		int len;
Signature · 1 line
1.1.4.g3b65
#5 lore raw

Re: [PATCH] rev-{list,parse}: optionally allow -<n> as shorthand for --max-count=<n>

Quoted 1 line · Eric Wong

This will only be enabled if POSIX_SHMOSIX is defined at compile-time.

Maybe a better name would be POSIX_ME_HARDER ^W oops, POSIXLY_CORRECT to disable this?

author
#6 lore raw

PATCH 1/1 rev-{list,parse}: allow -<n> as shorthand for --max-count=<n>

This will be enabled unless POSIXLY_CORRECT or POSIX_ME_HARDER is set in the environment.

Some versions of head(1) and tail(1) allow their line limits to be parsed this way. I find --max-count to be a commonly used option, and also similar in spirit to head/tail, so I decided to make life easier on my worn out (and lazy :) fingers with this patch.

---

 git-compat-util.h |    8 ++++++++
 rev-list.c        |    5 +++++
 rev-parse.c       |    4 ++++
 3 files changed, 17 insertions(+), 0 deletions(-)

7a8d6dda7dacadc90377fc56d28d5d03493199c4

git-compat-util.h +8 -0
index f982b8e..b2512ba 100644
@@ -154,4 +154,12 @@ static inline int sane_case(int x, int h
 #ifndef MAXPATHLEN
 #define MAXPATHLEN 256
 #endif
+
+static inline int posixly_correct(void)
+{
+	if (getenv("POSIXLY_CORRECT") || getenv("POSIX_ME_HARDER"))
+		return 1;
+	return 0;
+}
+
 #endif
rev-list.c +5 -0
index 4565755..f29d32a 100644
@@ -749,6 +749,11 @@ int main(int argc, const char **argv)
 		struct commit *commit;
 		unsigned char sha1[20];
 
+		/* accept, -<digit>, like some versions of head/tail  */
+		if (!posixly_correct() && (*arg == '-') && isdigit(arg[1])) {
+			max_count = atoi(arg + 1);
+			continue;
+		}
 		if (!strcmp(arg, "-n")) {
 			if (++i >= argc)
 				die("-n requires an argument");
rev-parse.c +4 -0
index 3c99a79..2966a33 100644
@@ -53,6 +53,10 @@ static int is_rev_argument(const char *a
 	};
 	const char **p = rev_args;
 
+	/* accept, -<digit>, like some versions of head/tail  */
+	if (!posixly_correct() && (*arg == '-') && isdigit(arg[1]))
+		return 1;
+
 	for (;;) {
 		const char *str = *p++;
 		int len;
Signature · 1 line
1.1.5.ga7f5-dirty
#7 lore raw

Re: [PATCH] rev-{list,parse}: allow -n<n> as shorthand for --max-count=<n>

In reply to Eric Wong
Quoted 5 lines · Eric Wong

Both -n<n> and -n <n> are supported. POSIX versions of head(1) and tail(1) allow their line limits to be parsed this way. I find --max-count to be a commonly used option, and also similar in spirit to head/tail, so I decided to make life easier on my worn out (and lazy :) fingers with this patch.

I agree with this in principle.

rev-parse.c static char *def = NULL; · line 24

Quoted 9 lines
rev-parse.c +1 -0
index 7abad35..3790463 100644
@@ -21,6 +21,7 @@ static char *def = NULL;
 static int show_type = NORMAL;
 static int symbolic = 0;
 static int output_sq = 0;
+static int next_arg_is_rev = 0;

Do you need this here, or can it be made auto in main()?

author
#8 lore raw

Re: [PATCH] rev-{list,parse}: allow -n<n> as shorthand for --max-count=<n>

Quoted 10 lines · Junio C Hamano
Quoted 5 lines · Eric Wong

Both -n<n> and -n <n> are supported. POSIX versions of head(1) and tail(1) allow their line limits to be parsed this way. I find --max-count to be a commonly used option, and also similar in spirit to head/tail, so I decided to make life easier on my worn out (and lazy :) fingers with this patch.

I agree with this in principle.

Quoted 11 lines

rev-parse.c static char *def = NULL; · line 24

Quoted 9 lines
rev-parse.c +1 -0
index 7abad35..3790463 100644
@@ -21,6 +21,7 @@ static char *def = NULL;
 static int show_type = NORMAL;
 static int symbolic = 0;
 static int output_sq = 0;
+static int next_arg_is_rev = 0;

Do you need this here, or can it be made auto in main()?

Oops, I had changes in is_rev_argument() that I eventually moved entirely to main(). Cleaned up patch on the way.

Signature · 1 line
Eric Wong
author
#9 lore raw

PATCH 1/1 rev-{list,parse}: allow -n<n> as shorthand for --max-count=<n>

Both -n<n> and -n <n> are supported. POSIX versions of head(1) and tail(1) allow their line limits to be parsed this way. I find --max-count to be a commonly used option, and also similar in spirit to head/tail, so I decided to make life easier on my worn out (and lazy :) fingers with this patch.

---

 rev-list.c  |   10 ++++++++++
 rev-parse.c |   15 +++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

a7f5327966f8ccdf887f3c63db3147135ff20983

rev-list.c +10 -0
index 0b142c1..4565755 100644
@@ -749,6 +749,16 @@ int main(int argc, const char **argv)
 		struct commit *commit;
 		unsigned char sha1[20];
 
+		if (!strcmp(arg, "-n")) {
+			if (++i >= argc)
+				die("-n requires an argument");
+			max_count = atoi(argv[i]);
+			continue;
+		}
+		if (!strncmp(arg,"-n",2)) {
+			max_count = atoi(arg + 2);
+			continue;
+		}
 		if (!strncmp(arg, "--max-count=", 12)) {
 			max_count = atoi(arg + 12);
 			continue;
rev-parse.c +15 -0
index d2f0864..3c99a79 100644
@@ -165,6 +165,21 @@ int main(int argc, char **argv)
 			show_file(arg);
 			continue;
 		}
+		if (!strcmp(arg,"-n")) {
+			if (++i >= argc)
+				die("-n requires an argument");
+			if ((filter & DO_FLAGS) && (filter & DO_REVS)) {
+				show(arg);
+				show(argv[i]);
+			}
+			continue;
+		}
+		if (!strncmp(arg,"-n",2)) {
+			if ((filter & DO_FLAGS) && (filter & DO_REVS))
+				show(arg);
+			continue;
+		}
+
 		if (*arg == '-') {
 			if (!strcmp(arg, "--")) {
 				as_is = 1;
Signature · 1 line
1.1.5.ga7f5-dirty
#10 lore raw
In reply to Eric Wong
Quoted 2 lines · Eric Wong

I don't agree with POSIX on this point, and I don't see why git should be bound to POSIX, especially at the UI level just because it's POSIX.

Unfortunately, it does not matter in the real world that you and I both do not particularly like "head -n 4". We will see more people who feel "-n 4" more familiar than "-4", unlike old fashioned people like myself.

Quoted 2 lines

... Heck, I've
been wanting GNU getopt_long() option parsing in git for a while...

We find somebody who wants to do this every now and then, it seems. Last time somebody brought this up in late May 2005, we were still a "too rapidly moving" target, adding and changing options every other day, and the actual implementation went nowhere while the discussion was reasonably healthy. If I recall the discussion correctly, argp instead of GNU getopt was the list favorite back then...