threads / patch / 56461

patch, 2 partsAdd cmd_gud and detect libiconv path for Mac OS

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

## tl;dr

6 messages between Sep 8, 2021 and Sep 8, 2021. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Colin Curtis· Sep 8, 2021, 05:13 UTC · lore
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)
Colin Curtis· Sep 8, 2021, 05:13 UTC · re: Colin Curtis · lore

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

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
Show changes to 7 files +81 −1

.gitignore, Documentation/git-gud.txt, Makefile, builtin.h, builtin/gud.c, git.c, 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)
Colin Curtis· Sep 8, 2021, 05:13 UTC · re: Colin Curtis · lore

[PATCH 2/2] add liconv link for makefile

From: Colin Curtis <colinpcurtis@gmail.com>
---
 Makefile      | 8 ++++++--
 builtin/gud.c | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)
Show changes to 2 files +7 −3

Makefile, builtin/gud.c

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)
Carlo Arenas· Sep 8, 2021, 05:52 UTC · re: Colin Curtis · lore

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

On Tue, Sep 7, 2021 at 10:18 PM Colin Curtis <colinpcurtis@gmail.com> wrote:
Show 10 quoted lines
>
> 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

Show 13 quoted lines
>  # 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
Bagas Sanjaya· Sep 8, 2021, 06:01 UTC · re: Colin Curtis · lore

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

On 08/09/21 12.13, Colin Curtis wrote:
Show 13 quoted lines
> 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
Ævar Arnfjörð Bjarmason· Sep 8, 2021, 10:50 UTC · re: Colin Curtis · lore

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

On Tue, Sep 07 2021, Colin Curtis wrote:
Show 7 quoted lines
> 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.

← back to recent threads