[PATCH 0/2] Add cmd_gud and detect libiconv path for Mac OS

[PATCH 0/2] Add cmd_gud and detect libiconv path for Mac OS

[Colin Curtis]

From: Colin Curtis <colinpcurtis@gmail.com>

The gud command opens the Pro Git book webpage in the default
web browser.  The reason to add this command is due to the
play on words when saying 'git gud', which sounds like 'get good'.
Hence this command when invoked will open up the Pro Git 
webpage to allow the user to 'get good' at git.

We also fix a bug in the Makefile when running on Mac OS, namely
the libiconv path when using a brew install for the library.  Previously
the developer would have to manually change the path to the library when
developing on Mac OS.

Colin Curtis (2):
  add cmd_gud to open git-scm.com webpage
  add liconv link for makefile

 .gitignore                |  1 +
 Documentation/git-gud.txt | 33 +++++++++++++++++++++++++++++++++
 Makefile                  |  9 +++++++--
 builtin.h                 |  1 +
 builtin/gud.c             | 27 +++++++++++++++++++++++++++
 git.c                     |  1 +
 t/t9904-git-gud.sh        | 16 ++++++++++++++++
 7 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/git-gud.txt
 create mode 100644 builtin/gud.c
 create mode 100755 t/t9904-git-gud.sh

-- 
2.30.1 (Apple Git-130)

[PATCH 1/2] add cmd_gud to open git-scm.com webpage

[Colin Curtis]

From: Colin Curtis <colinpcurtis@gmail.com>

---
 .gitignore                |  1 +
 Documentation/git-gud.txt | 33 +++++++++++++++++++++++++++++++++
 Makefile                  |  3 ++-
 builtin.h                 |  1 +
 builtin/gud.c             | 27 +++++++++++++++++++++++++++
 git.c                     |  1 +
 t/t9904-git-gud.sh        | 16 ++++++++++++++++
 7 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/git-gud.txt
 create mode 100644 builtin/gud.c
 create mode 100755 t/t9904-git-gud.sh

diff --git a/.gitignore b/.gitignore
index 311841f9be..5fec146bd8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -75,6 +75,7 @@
 /git-gc
 /git-get-tar-commit-id
 /git-grep
+/git-gud
 /git-hash-object
 /git-help
 /git-http-backend
diff --git a/Documentation/git-gud.txt b/Documentation/git-gud.txt
new file mode 100644
index 0000000000..357ff915a6
--- /dev/null
+++ b/Documentation/git-gud.txt
@@ -0,0 +1,33 @@
+git-gud(1)
+===========
+
+NAME
+----
+git-gud - Display git-scm.com website
+
+SYNOPSIS
+--------
+[verse]
+'git gud' [-d | --display] [<pathspec>...]
+
+DESCRIPTION
+-----------
+
+'git gud' command opens the webpage for git-scm.com in the default
+web browser.
+
+OPTIONS
+-------
+-d::
+--display::
+	Opens the webpage for git-scm.com in the default browser
+
+OUTPUT
+------
+If the '[-d | --display]' option is present, then the command opens
+up the git-scm.com webpage in the default web browser.  Otherwise,
+nothing is done.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 429c276058..379cd91a97 100644
--- a/Makefile
+++ b/Makefile
@@ -1108,6 +1108,7 @@ BUILTIN_OBJS += builtin/fsck.o
 BUILTIN_OBJS += builtin/gc.o
 BUILTIN_OBJS += builtin/get-tar-commit-id.o
 BUILTIN_OBJS += builtin/grep.o
+BUILTIN_OBJS += builtin/gud.o
 BUILTIN_OBJS += builtin/hash-object.o
 BUILTIN_OBJS += builtin/help.o
 BUILTIN_OBJS += builtin/index-pack.o
@@ -1513,7 +1514,7 @@ ifndef NO_ICONV
 		ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
 			ICONV_LINK += -lintl
 		endif
-		EXTLIBS += $(ICONV_LINK) -liconv
+		EXTLIBS += $(ICONV_LINK) /usr/local/Cellar/libiconv/1.16/lib/libiconv.dylib # -liconv
 	endif
 endif
 ifdef ICONV_OMITS_BOM
diff --git a/builtin.h b/builtin.h
index 16ecd5586f..e183a1a8d4 100644
--- a/builtin.h
+++ b/builtin.h
@@ -162,6 +162,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix);
 int cmd_gc(int argc, const char **argv, const char *prefix);
 int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);
 int cmd_grep(int argc, const char **argv, const char *prefix);
+int cmd_gud(int argc, const char **argv, const char *prefix);
 int cmd_hash_object(int argc, const char **argv, const char *prefix);
 int cmd_help(int argc, const char **argv, const char *prefix);
 int cmd_index_pack(int argc, const char **argv, const char *prefix);
diff --git a/builtin/gud.c b/builtin/gud.c
new file mode 100644
index 0000000000..04808a08f5
--- /dev/null
+++ b/builtin/gud.c
@@ -0,0 +1,27 @@
+#include <stdio.h>
+#include "builtin.h"
+#include "parse-options.h"
+
+static int display_page;
+
+static const char * const builtin_gud_usage[] = {
+	N_("git gud [<options>]"),
+	NULL
+};
+
+static struct option builtin_gud_options[] = {
+        OPT_BOOL('d', "display", &display_page, N_("display the webpage for git-scm.com")),
+		OPT_END()
+};
+
+int cmd_gud(int argc, const char **argv, const char *prefix)
+{    
+
+    argc = parse_options(argc, argv, prefix,
+                builtin_gud_options, builtin_gud_usage, 0);
+    if (display_page) {
+        system("open https://git-scm.com/book/en/v2");
+    }
+    
+    return 0;
+}
\ No newline at end of file
diff --git a/git.c b/git.c
index 18bed9a996..2da1f4d2d4 100644
--- a/git.c
+++ b/git.c
@@ -536,6 +536,7 @@ static struct cmd_struct commands[] = {
 	{ "gc", cmd_gc, RUN_SETUP },
 	{ "get-tar-commit-id", cmd_get_tar_commit_id, NO_PARSEOPT },
 	{ "grep", cmd_grep, RUN_SETUP_GENTLY },
+	{ "gud", cmd_gud, RUN_SETUP },
 	{ "hash-object", cmd_hash_object },
 	{ "help", cmd_help },
 	{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },
diff --git a/t/t9904-git-gud.sh b/t/t9904-git-gud.sh
new file mode 100755
index 0000000000..47fa97498c
--- /dev/null
+++ b/t/t9904-git-gud.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+test_description='git-gud test
+
+This test runs git-gud and makes sure it does not crash.'
+
+. ./test-lib.sh
+
+test_expect_success 'runs correctly with no args' '
+	git gud
+'
+
+test_expect_success 'runs correctly with -d option' '
+	git gud -d
+'
+
+test_done
-- 
2.30.1 (Apple Git-130)

[PATCH 2/2] add liconv link for makefile

[Colin Curtis]

From: Colin Curtis <colinpcurtis@gmail.com>

---
 Makefile      | 8 ++++++--
 builtin/gud.c | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 379cd91a97..e1679cca47 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@
 # The default target of this Makefile is...
 all::
-
+OS := $(shell uname)
 # Define V=1 to have a more verbose compile.
 #
 # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
@@ -1514,7 +1514,11 @@ ifndef NO_ICONV
 		ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
 			ICONV_LINK += -lintl
 		endif
-		EXTLIBS += $(ICONV_LINK) /usr/local/Cellar/libiconv/1.16/lib/libiconv.dylib # -liconv
+		ifeq ($(OS),Darwin)
+			EXTLIBS += $(ICONV_LINK) /usr/local/Cellar/libiconv/1.16/lib/libiconv.dylib
+		else
+			EXTLIBS += $(ICONV_LINK) -liconv
+		endif
 	endif
 endif
 ifdef ICONV_OMITS_BOM
diff --git a/builtin/gud.c b/builtin/gud.c
index 04808a08f5..9a5a1e71ac 100644
--- a/builtin/gud.c
+++ b/builtin/gud.c
@@ -24,4 +24,4 @@ int cmd_gud(int argc, const char **argv, const char *prefix)
     }
     
     return 0;
-}
\ No newline at end of file
+}
-- 
2.30.1 (Apple Git-130)

Re: [PATCH 2/2] add liconv link for makefile

[Carlo Arenas]

On Tue, Sep 7, 2021 at 10:18 PM Colin Curtis <colinpcurtis@gmail.com> wrote:
>
> diff --git a/Makefile b/Makefile
> index 379cd91a97..e1679cca47 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,6 +1,6 @@
>  # The default target of this Makefile is...
>  all::
> -
> +OS := $(shell uname)

There is no need for this, the section of code you modify below is
already macOS (indeed, even to the point that it won't trigger in a
Linux user using brew, or even a macOS user that has macports)
specific

>  # Define V=1 to have a more verbose compile.
>  #
>  # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
> @@ -1514,7 +1514,11 @@ ifndef NO_ICONV
>                 ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
>                         ICONV_LINK += -lintl
>                 endif
> -               EXTLIBS += $(ICONV_LINK) /usr/local/Cellar/libiconv/1.16/lib/libiconv.dylib # -liconv
> +               ifeq ($(OS),Darwin)
> +                       EXTLIBS += $(ICONV_LINK) /usr/local/Cellar/libiconv/1.16/lib/libiconv.dylib
> +               else
> +                       EXTLIBS += $(ICONV_LINK) -liconv
> +               endif

Why is it not built with the libiconv library that is provided by the system?

$ otool -L git
git:
        /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
(compatibility version 1.0.0, current version 1122.33.0)
        /usr/lib/libz.1.dylib (compatibility version 1.0.0, current
version 1.2.11)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0,
current version 7.0.0)
        /usr/local/opt/gettext/lib/libintl.8.dylib (compatibility
version 11.0.0, current version 11.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0,
current version 1292.100.5)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
(compatibility version 150.0.0, current version 1775.118.101)

AFAIK there is a good reason why brew doesn't link that automatically,
and using the headers of one with the binary of the other is likely to
cause serious problems.

Carlo

Re: [PATCH 0/2] Add cmd_gud and detect libiconv path for Mac OS

[Bagas Sanjaya]

On 08/09/21 12.13, Colin Curtis wrote:
> From: Colin Curtis <colinpcurtis@gmail.com>
> 
> The gud command opens the Pro Git book webpage in the default
> web browser.  The reason to add this command is due to the
> play on words when saying 'git gud', which sounds like 'get good'.
> Hence this command when invoked will open up the Pro Git
> webpage to allow the user to 'get good' at git.
> 
> We also fix a bug in the Makefile when running on Mac OS, namely
> the libiconv path when using a brew install for the library.  Previously
> the developer would have to manually change the path to the library when
> developing on Mac OS.
> 

Two distinct topics in one patch series. Why don't you create separate 
patch series for each topic?

For `git gud`, I think there's no point on adding such easter egg, since 
users can simply access git-scm.com with their browser, without needing 
specialized command just to invoke it.

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH 0/2] Add cmd_gud and detect libiconv path for Mac OS

[Ævar Arnfjörð Bjarmason]

On Tue, Sep 07 2021, Colin Curtis wrote:

> From: Colin Curtis <colinpcurtis@gmail.com>
>
> The gud command opens the Pro Git book webpage in the default
> web browser.  The reason to add this command is due to the
> play on words when saying 'git gud', which sounds like 'get good'.
> Hence this command when invoked will open up the Pro Git 
> webpage to allow the user to 'get good' at git.

Purely in terms of implementation if we had this sort of thing it really
would belong in "git help", not in an overly cleverly named new
built-in.

More generally we don't link to git-scm.org now for anything
significant, AFAICT the only things we do link to are to our own
generated documentation.

[Not with my Git PLC hat on, in case anyone's wondering]

I don't think we should be further endorsing proprietary documentation
in liue of improving the free docs in git.git itself.

If this command (whether via "git help" or not) linked to anything it
should be to our own https://git-scm.com/docs/user-manual; if there's
things lacking there let's try to improve the freely available docs.

And purely in terms of UX once we had such freely available docs we
should not be opening a webpage to display them, but linking to a
manpage etc. Perhaps we should be opening a webpage, and indeed "git
help" can do that for you, but that should be to docs we have built
locally & are guaranteed to apply to the git version you're working
with.