let set diff-highlight=true work when not on $PATH

* diff-highlight is probably not on the user's $PATH in most installs * find it relative to git --exec-path * also rescue the case where diff-highlight isn't a prepared executable
jonas · Aug 5, 2017 · 26a8f6e · 26a8f6e
1 parent 29a8da2
commit 26a8f6e
Show file tree

Hide file tree

Showing 4 changed files with 200 additions and 83 deletions.
diff --git a/include/tig/apps.h b/include/tig/apps.h
@@ -18,6 +18,21 @@
 #include "tig/argv.h"
 #include "tig/util.h"
 
+/*
+ * general
+ */
+
+struct app_external {
+	const char *argv[SIZEOF_ARG];
+	char * const env[SIZEOF_ARG];
+};
+
+/*
+ * diff-highlight
+ */
+
+struct app_external *app_diff_highlight_load(const char *query);
+
 #endif
 
 /* vim: set ts=8 sw=8 noexpandtab: */
diff --git a/src/apps.c b/src/apps.c
@@ -15,4 +15,111 @@
 #include "tig/io.h"
 #include "tig/apps.h"
 
+/*
+ * general
+ */
+
+static bool
+app_oneline_buf(char *buf, size_t bufsize, struct app_external *app, const char *dir)
+{
+	struct io io;
+	return io_run(&io, IO_RD, dir, app->env, app->argv) \
+		&& io_read_buf(&io, buf, bufsize, false);
+}
+
+/*
+ * git
+ */
+
+static bool
+app_git_exec_path(char *path, size_t path_len)
+{
+	static char exec_path[SIZEOF_STR] = "";
+	struct app_external app = {
+		{ "git", "--exec-path", NULL },
+		{ "GIT_CONFIG=/dev/null", NULL },
+	};
+
+	if (!*exec_path)
+		app_oneline_buf(exec_path, sizeof(exec_path), &app, NULL);
+
+	if (!*exec_path)
+		return false;
+
+	string_ncopy_do(path, path_len, exec_path, sizeof(exec_path));
+	return true;
+}
+
+/*
+ * diff-highlight
+ */
+
+static bool
+app_diff_highlight_path_search(char *dest, size_t destlen, const char *query)
+{
+	if (!query || !*query)
+		return false;
+
+	if (strchr(query, '/')) {
+		/* can only be interpreted as a fully qualified path */
+		string_ncopy_do(dest, destlen, query, strlen(query));
+		return true;
+	}
+
+	const char *env_path = getenv("PATH");
+	char env_path_plus[SIZEOF_MED_STR];
+	char exec_path[SIZEOF_STR];
+
+	if (!env_path || !*env_path)
+		env_path = _PATH_DEFPATH;
+
+	if (app_git_exec_path(exec_path, sizeof(exec_path)))
+		string_format(env_path_plus, "%s:%s/%s/%s:%s/%s/%s:%s/%s/%s:%s/%s/%s",
+			      env_path,
+			      exec_path, "../../share/git-core/contrib", query,
+			      exec_path, "../share/git-core/contrib", query,
+			      exec_path, "../../share/git/contrib", query,
+			      exec_path, "../share/git/contrib", query);
+	else
+		string_ncopy(env_path_plus, env_path, strlen(env_path));
+
+	if (!path_search(dest, destlen, query, env_path_plus, X_OK)
+	    && !strcmp(query, "diff-highlight")
+	    && !path_search(dest, destlen, "diff-highlight.perl", env_path_plus, R_OK))
+		return false;
+
+	return true;
+}
+
+struct app_external
+*app_diff_highlight_load(const char *query)
+{
+	static struct app_external dhlt_app = { { NULL }, { "GIT_CONFIG=/dev/null", NULL } };
+	static bool did_search = false;
+	static char dhlt_path[SIZEOF_STR];
+	static char perl_path[SIZEOF_STR];
+	static char perl_include[SIZEOF_STR];
+
+	if (!did_search
+	    && app_diff_highlight_path_search(dhlt_path, sizeof(dhlt_path), query)
+	    && *dhlt_path) {
+		if (suffixcmp(dhlt_path, strlen(dhlt_path), "/diff-highlight.perl")) {
+			dhlt_app.argv[0] = dhlt_path;
+			dhlt_app.argv[1] = NULL;
+		} else if (path_search(perl_path, sizeof(perl_path), "perl", getenv("PATH"), X_OK)) {
+			/* if the package manager failed to "make install" within the contrib dir, rescue via */
+			/* perl -MDiffHighlight -I/path/containing /path/containing/diff-highlight.perl */
+			string_format(perl_include, "-I%s", dirname(dhlt_path));
+			dhlt_app.argv[0] = perl_path;
+			dhlt_app.argv[1] = "-MDiffHighlight";
+			dhlt_app.argv[2] = perl_include;
+			dhlt_app.argv[3] = dhlt_path;
+			dhlt_app.argv[4] = NULL;
+		}
+	}
+	did_search = true;
+
+	return &dhlt_app;
+}
+
 /* vim: set ts=8 sw=8 noexpandtab: */
diff --git a/src/diff.c b/src/diff.c
@@ -20,6 +20,7 @@
 #include "tig/pager.h"
 #include "tig/diff.h"
 #include "tig/draw.h"
+#include "tig/apps.h"
 
 static enum status_code
 diff_open(struct view *view, enum open_flags flags)
@@ -45,18 +46,18 @@ diff_open(struct view *view, enum open_flags flags)
 enum status_code
 diff_init_highlight(struct view *view, struct diff_state *state)
 {
-	if (opt_diff_highlight) {
-		const char *argv[] = { opt_diff_highlight, NULL };
-		char * const env[] = { "GIT_CONFIG=/dev/null", NULL };
-		struct io io;
+	if (!opt_diff_highlight || !*opt_diff_highlight)
+		return SUCCESS;
 
-		if (!io_exec(&io, IO_RP, view->dir, env, argv, view->io.pipe))
-			return error("Failed to run %s", opt_diff_highlight);
+	struct app_external *app = app_diff_highlight_load(opt_diff_highlight);
+	struct io io;
 
-		state->view_io = view->io;
-		view->io = io;
-		state->highlight = true;
-	}
+	if (!io_exec(&io, IO_RP, view->dir, app->env, app->argv, view->io.pipe))
+		return error("Failed to run %s", opt_diff_highlight);
+
+	state->view_io = view->io;
+	view->io = io;
+	state->highlight = true;
 
 	return SUCCESS;
 }

diff --git a/test/diff/diff-highlight-test b/test/diff/diff-highlight-test
@@ -3,34 +3,20 @@
 . libtest.sh
 . libgit.sh
 
+in_work_dir create_repo_from_tgz "$base_dir/files/scala-js-benchmarks.tgz"
+
 file bin/diff-highlight <<EOF
 #!/bin/sh
 exec sed 's/Add type parameter for js.Dynamic/Changed by diff-highlight/'
 EOF
 
 chmod +x bin/diff-highlight
 
-tigrc <<EOF
-set diff-highlight = true
-EOF
-
-export PATH="./bin:$PATH"
-
-steps '
-	:save-display diff-default.screen
-
-	:set diff-highlight = cat
-	:save-display diff-custom.screen
-
-	:set diff-highlight = program-that-does-not-exist
-	:save-display diff-failure.screen
-'
-
-in_work_dir create_repo_from_tgz "$base_dir/files/scala-js-benchmarks.tgz"
-
-test_tig show master^
-
-assert_equals 'diff-default.screen' <<EOF
+test_case diff-highlight-true \
+	--tigrc='set diff-highlight = true' \
+	--args='show master^' \
+	--subshell='export PATH="./bin:$PATH"' \
+	<<EOF
 commit a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff
 Author:     Jonas Fonseca <jonas.fonseca@gmail.com>
 AuthorDate: Sat Mar 1 15:59:02 2014 -0500
@@ -62,66 +48,74 @@ index 65f914a..3aa4320 100644
 [diff] a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff - line 1 of 24              100%
 EOF
 
-assert_equals 'diff-custom.screen' <<EOF
-commit a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff
-Author:     Jonas Fonseca <jonas.fonseca@gmail.com>
-AuthorDate: Sat Mar 1 15:59:02 2014 -0500
-Commit:     Jonas Fonseca <jonas.fonseca@gmail.com>
-CommitDate: Sat Mar 1 15:59:02 2014 -0500
+test_case diff-highlight-custom \
+	--tigrc='set diff-highlight = wc' \
+	--args='show master^' \
+	<<EOF
+24      85    1067
 
-    Add type parameter for js.Dynamic
----
- common/src/main/scala/org/scalajs/benchmark/Benchmark.scala | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
 
-diff --git a/common/src/main/scala/org/scalajs/benchmark/Benchmark.scala b/commo
-index 65f914a..3aa4320 100644
---- a/common/src/main/scala/org/scalajs/benchmark/Benchmark.scala
-+++ b/common/src/main/scala/org/scalajs/benchmark/Benchmark.scala
-@@ -15,7 +15,7 @@ object Benchmark {
-   val benchmarks = js.Array[Benchmark]()
-   val benchmarkApps = js.Array[BenchmarkApp]()
 
--  val global = js.Dynamic.global.asInstanceOf[js.Dictionary]
-+  val global = js.Dynamic.global.asInstanceOf[js.Dictionary[js.Any]]
-   global("runScalaJSBenchmarks") = runBenchmarks _
-   global("initScalaJSBenchmarkApps") = initBenchmarkApps _
 
 
 
 
 
-[diff] a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff - line 1 of 24              100%
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+[diff] HEAD - line 1 of 1                                                   100%
 EOF
 
-assert_equals 'diff-failure.screen' <<EOF
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
-[diff] a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff                               0%
+test_case diff-highlight-misconfigured \
+	--tigrc='set diff-highlight = program-that-does-not-exist' \
+	--args='show master^' \
+	<<EOF
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+[diff]                                                                        0%
 EOF
+
+run_test_cases