Rework after review

koutcher · Nov 8, 2023 · 36cb524 · 36cb524
1 parent fc1a0cb
commit 36cb524
Show file tree

Hide file tree

Showing 20 changed files with 275 additions and 33 deletions.
diff --git a/doc/manual.adoc b/doc/manual.adoc
@@ -203,7 +203,7 @@ following variables.
 |%(repo:is-inside-work-tree)
 			|Whether Tig is running inside a work tree,
 			 either `true` or `false`.
-|%(repo:hash_len)	|The hash algorithm used for the repository, e.g. `sha1`
+|%(repo:object-format)	|The hash algorithm used for the repository, e.g. `sha1`
 			 or `sha256`.
 |=============================================================================
 

diff --git a/include/tig/refdb.h b/include/tig/refdb.h
@@ -17,6 +17,7 @@
 #include "tig/tig.h"
 #include "tig/types.h"
 #include "tig/util.h"
+#include "tig/repo.h"
 
 struct argv_env;
 
@@ -29,7 +30,7 @@ struct ref {
 };
 
 #define is_initial_commit()	(!get_ref_head())
-#define is_head_commit(rev)	(!strcmp((rev), "HEAD") || (get_ref_head() && !strncmp(rev, get_ref_head()->id, repo.hash_len)))
+#define is_head_commit(rev)	(!strcmp((rev), "HEAD") || (get_ref_head() && !strncmp(rev, get_ref_head()->id, REPO_SIZEOF_REV - 1)))
 #define ref_is_tag(ref)		((ref)->type == REFERENCE_TAG || (ref)->type == REFERENCE_LOCAL_TAG)
 #define ref_is_remote(ref)	((ref)->type == REFERENCE_REMOTE || (ref)->type == REFERENCE_TRACKED_REMOTE)
 

diff --git a/include/tig/repo.h b/include/tig/repo.h
@@ -16,13 +16,20 @@
 
 #include "tig/tig.h"
 
-typedef int repo_hash_len;
+typedef enum {
+	REPO_SHA1 = 40,
+	REPO_SHA256 = 64
+} repo_object_format;
+
+#define REPO_SIZEOF_REV	(repo.object_format + 1)
+
 typedef char repo_ref[SIZEOF_REF];
 typedef char repo_rev[SIZEOF_REV];
 typedef char repo_str[SIZEOF_STR];
 
+/* Leave object_format in first position. */
 #define REPO_INFO(_) \
-	_(repo_hash_len, hash_len) \
+	_(repo_object_format, object_format) \
 	_(repo_ref, head) \
 	_(repo_rev, head_id) \
 	_(repo_ref, remote) \

diff --git a/src/argv.c b/src/argv.c
@@ -416,11 +416,11 @@ bool_formatter(struct format_context *format, struct format_var *var)
 }
 
 static bool
-repo_hash_len_formatter(struct format_context *format, struct format_var *var)
+repo_object_format_formatter(struct format_context *format, struct format_var *var)
 {
-	int value = *(int *)var->value_ref;
+	repo_object_format value = *(repo_object_format *)var->value_ref;
 
-	return string_format_from(format->buf, &format->bufpos, "%s", value == 64 ? "sha256" : "sha1");
+	return string_format_from(format->buf, &format->bufpos, "%s", value == REPO_SHA256 ? "sha256" : "sha1");
 }
 
 static bool

diff --git a/src/blame.c b/src/blame.c
@@ -187,15 +187,15 @@ get_blame_commit(struct view *view, const char *id)
 		if (!blame->commit)
 			continue;
 
-		if (!strncmp(blame->commit->id, id, repo.hash_len))
+		if (!strncmp(blame->commit->id, id, REPO_SIZEOF_REV - 1))
 			return blame->commit;
 	}
 
 	{
 		struct blame_commit *commit = calloc(1, sizeof(*commit));
 
 		if (commit)
-			string_ncopy(commit->id, id, repo.hash_len + 1);
+			string_ncopy(commit->id, id, REPO_SIZEOF_REV);
 		return commit;
 	}
 }

diff --git a/src/draw.c b/src/draw.c
@@ -268,7 +268,7 @@ draw_id(struct view *view, struct view_column *column, const char *id)
 		return false;
 
 	if (column->opt.id.color && id) {
-		hashval_t color = iterative_hash(id, repo.hash_len, 0);
+		hashval_t color = iterative_hash(id, REPO_SIZEOF_REV - 1, 0);
 
 		type = palette_colors[color % ARRAY_SIZE(palette_colors)];
 	}

diff --git a/src/main.c b/src/main.c
@@ -101,7 +101,7 @@ main_add_commit(struct view *view, enum line_type type, struct commit *template,
 	view_column_info_update(view, line);
 
 	if ((opt_start_on_head && is_head_commit(commit->id)) ||
-	    (view->env->goto_id[0] && !strncmp(view->env->goto_id, commit->id, repo.hash_len)))
+	    (view->env->goto_id[0] && !strncmp(view->env->goto_id, commit->id, REPO_SIZEOF_REV - 1)))
 		select_view_line(view, line->lineno + 1);
 
 	return commit;

diff --git a/src/options.c b/src/options.c
@@ -701,9 +701,16 @@ parse_option(struct option_info *option, const char *prefix, const char *arg)
 		if (!strcmp(name, "line-number-interval") ||
 		    !strcmp(name, "tab-size"))
 			return parse_int(option->value, arg, 1, 1024);
-		else if (!strcmp(name, "id-width"))
-			return parse_int(option->value, arg, 0, SIZEOF_REV - 1);
-		else
+		else if (!strcmp(name, "id-width")) {
+			enum status_code code = parse_int(option->value, arg, 0, SIZEOF_REV - 1);
+			if (code == SUCCESS) {
+				int *value = option->value;
+				/* Limit id-width to the length of the hash used for the repository. */
+				if (*value > REPO_SIZEOF_REV - 1)
+					*value = REPO_SIZEOF_REV - 1;
+			}
+			return code;
+		} else
 			return parse_int(option->value, arg, 0, 1024);
 	}
 
@@ -1528,11 +1535,16 @@ read_repo_config_option(char *name, size_t namelen, char *value, size_t valuelen
 	else if (!strcmp(name, "core.worktree"))
 		string_ncopy(repo.worktree, value, valuelen);
 
-	else if (!strcmp(name, "core.abbrev"))
+	else if (!strcmp(name, "core.abbrev")) {
+		/* We cannot use REPO_SIZEOF_REV until we parse extensions.objectformat. */
+		if (!strcmp(value, "no")) {
+			opt_id_width = SIZEOF_REV - 1;
+			return SUCCESS;
+		}
 		parse_int(&opt_id_width, value, 0, SIZEOF_REV - 1);
 
-	else if (!strcmp(name, "extensions.objectformat"))
-		repo.hash_len = !strcmp(value, "sha256") ? 64 : 40;
+	} else if (!strcmp(name, "extensions.objectformat"))
+		repo.object_format = !strcmp(value, "sha256") ? REPO_SHA256 : REPO_SHA1;
 
 	else if (!strcmp(name, "diff.noprefix"))
 		parse_bool(&opt_diff_noprefix, value);
@@ -1581,6 +1593,10 @@ load_git_config(void)
 
 	code = io_run_load(&io, config_list_argv, "=", read_repo_config_option, NULL);
 
+	/* Limit id-width to the length of the hash used for the repository. */
+	if (opt_id_width > REPO_SIZEOF_REV - 1)
+		opt_id_width = REPO_SIZEOF_REV - 1;
+
 	if (git_worktree && *git_worktree)
 		string_ncopy(repo.worktree, git_worktree, strlen(git_worktree));
 

diff --git a/src/parse.c b/src/parse.c
@@ -110,9 +110,9 @@ parse_number(const char **posref, size_t *number)
 bool
 parse_blame_header(struct blame_header *header, const char *text)
 {
-	const char *pos = text + repo.hash_len - 1;
+	const char *pos = text + REPO_SIZEOF_REV - 2;
 
-	if (strlen(text) < repo.hash_len || pos[1] != ' ')
+	if (strlen(text) <= REPO_SIZEOF_REV || pos[1] != ' ')
 		return false;
 
 	string_copy_rev(header->id, text);
@@ -164,10 +164,10 @@ parse_blame_info(struct blame_commit *commit, char author[SIZEOF_STR], char *lin
 		string_ncopy(commit->title, line, strlen(line));
 
 	} else if (match_blame_header("previous ", &line)) {
-		if (strlen(line) < repo.hash_len)
+		if (strlen(line) <= REPO_SIZEOF_REV)
 			return false;
 		string_copy_rev(commit->parent_id, line);
-		line += repo.hash_len + 1;
+		line += REPO_SIZEOF_REV;
 		commit->parent_filename = get_path(line);
 		if (!commit->parent_filename)
 			return true;

diff --git a/src/refdb.c b/src/refdb.c
@@ -269,7 +269,7 @@ add_to_refs(const char *id, size_t idlen, char *name, size_t namelen, struct ref
 
 	ref->valid = true;
 	ref->type = type;
-	string_ncopy_do(ref->id, repo.hash_len + 1, id, idlen);
+	string_ncopy_do(ref->id, REPO_SIZEOF_REV, id, idlen);
 
 	if (type == REFERENCE_HEAD) {
 		if (!refs_head ||

diff --git a/src/repo.c b/src/repo.c
@@ -57,7 +57,7 @@ read_repo_info(char *name, size_t namelen, char *value, size_t valuelen, void *d
 		 * this special case by looking at the emitted value. If it looks
 		 * like a commit ID and there's no cdup path assume that no value
 		 * was emitted. */
-		if (!*repo.cdup && namelen == repo.hash_len && iscommit(name))
+		if (!*repo.cdup && namelen == REPO_SIZEOF_REV - 1 && iscommit(name))
 			return read_repo_info(name, namelen, value, valuelen, data);
 
 		string_ncopy(repo.prefix, name, namelen);
@@ -102,8 +102,8 @@ load_repo_info(void)
 	};
 
 	memset(&repo, 0, sizeof(repo));
-	/* defaults to SHA-1 as older Git versions don't have extensions.objectformat */
-	repo.hash_len = 40;
+	/* defaults to SHA-1 as older Git versions don't have extensions.objectFormat */
+	repo.object_format = REPO_SHA1;
 	return reload_repo_info(rev_parse_argv);
 }
 

diff --git a/src/string.c b/src/string.c
@@ -12,8 +12,8 @@
  */
 
 #include "tig/tig.h"
-#include "tig/repo.h"
 #include "tig/string.h"
+#include "tig/repo.h"
 #include "compat/utf8proc.h"
 
 /*
@@ -43,7 +43,7 @@ iscommit(const char *str)
 			return false;
 	}
 
-	return 7 <= pos && pos <= repo.hash_len;
+	return 7 <= pos && pos < REPO_SIZEOF_REV;
 }
 
 int
@@ -73,11 +73,11 @@ string_copy_rev(char *dst, const char *src)
 	if (!*src)
 		return;
 
-	for (srclen = 0; srclen <= repo.hash_len; srclen++)
+	for (srclen = 0; srclen < REPO_SIZEOF_REV; srclen++)
 		if (!src[srclen] || isspace((unsigned char)src[srclen]))
 			break;
 
-	string_ncopy_do(dst, repo.hash_len + 1, src, srclen);
+	string_ncopy_do(dst, REPO_SIZEOF_REV, src, srclen);
 }
 
 void

diff --git a/src/tree.c b/src/tree.c
@@ -58,7 +58,7 @@ push_tree_stack_entry(struct view *view, const char *name, struct position *posi
  */
 
 #define SIZEOF_TREE_ATTR \
-	STRING_SIZE("100644 blob ") + repo.hash_len + 1
+	STRING_SIZE("100644 blob ") + REPO_SIZEOF_REV - 1 + STRING_SIZE("\t")
 
 #define SIZEOF_TREE_MODE \
 	STRING_SIZE("100644 ")

diff --git a/test/blame/sha256-initial-diff-test b/test/blame/sha256-initial-diff-test
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+. libtest.sh
+. libgit.sh
+
+test_require sha256
+
+steps '
+	:save-display initial-diff.screen
+	:20 # Move to a deleted line.
+	:view-blame
+	:scroll-right
+	:save-display blame-deleted-line.screen
+'
+
+in_work_dir create_repo_from_tgz "$base_dir/files/sha256-scala-js-benchmarks.tgz"
+
+LINES=23 test_tig show ed3277a
+
+assert_equals 'initial-diff.screen' <<EOF
+commit ed3277a31a5a48def23854db7eaa7356871a29cbf26ba279cb8f1e4f93cf54fb
+Author:     Jonas Fonseca <[email protected]>
+AuthorDate: Sat Mar 1 15:59:02 2014 -0500
+Commit:     Jonas Fonseca <[email protected]>
+CommitDate: Sat Mar 1 15:59:02 2014 -0500
+
+    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 4abd433..aa90cdf 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]]
+[diff] ed3277a31a5a48def23854db7eaa7356871a29cbf26ba279cb8f1e4f93cf54fb - li 87%
+EOF
+
+# Make sure that we find the commit that introduce the deleted line.
+assert_equals 'blame-deleted-line.screen' <<EOF
+0400   1x /*                     __
+0400   2x **     ________ ___   / /  ___      __ ____  Scala.js Benchmarks
+0400   3x **    / __/ __// _ | / /  / _ | __ / // __/  (c) 2003-2013, LAMP/EPFL
+0400   4x **  __\ \/ /__/ __ |/ /__/ __ |/_// /_\ \    (c) 2013, Jonas Fonseca
+0400   5x ** /____/\___/_/ |_/____/_/ | |__/ /____/
+0400   6x **                          |/____/
+0400   7x \*
+0400   8x
+0400   9x package benchmarks
+0400  10x
+0400  11x import scala.compat.Platform
+0500  12x import scala.scalajs.js
+0500  13x
+0500  14x object Benchmark {
+0500  15x   val benchmarks = js.Array[js.Function0[Unit]]()
+0500  16x
+0500  17x   val global = js.Dynamic.global.asInstanceOf[js.Dictionary]
+0500  18x   global("ScalaJSBenchmarks") = benchmarks
+0500  19x
+0500  20x   def add(benchmark: Benchmark) {
+0500  21x     benchmarks.push {
+[blame] common/Benchmark.scala - line 17 of 99                               21%
+EOF
diff --git a/test/files/sha256-repo-one.tgz b/test/files/sha256-repo-one.tgz
diff --git a/test/files/sha256-scala-js-benchmarks.tgz b/test/files/sha256-scala-js-benchmarks.tgz