Re: [PATCH v3 3/4] commit.c: replace some literal strings with constants
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:50:35
Jay Soffian wrote:
I converted these per http://article.gmane.org/gmane.comp.version-control.git/167015 Maybe this should be the last patch in the series; it's questionable to me whether it's even worth doing.
What have I wrought? I think this makes the code much less readable.
quoted hunk ↗ jump to hunk
--- a/builtin/commit.c +++ b/builtin/commit.c@@ -626,13 +632,13 @@ static int prepare_to_commit(const char *index_file, const char *prefix, format_commit_message(commit, "fixup! %s\n\n", &sb, &ctx); hook_arg1 = "message"; - } else if (!stat(git_path("MERGE_MSG"), &statbuf)) { - if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0) - die_errno("could not read MERGE_MSG"); + } else if (!stat(git_path(merge_msg), &statbuf)) { + if (strbuf_read_file(&sb, git_path(merge_msg), 0) < 0) + die_errno("could not read %s", merge_msg);
So now a person needs to look earlier in the file to see that
merge_msg means "MERGE_MSG" rather than being something that
can vary.
Yes, commit_editmsg has the same problem. If I get to choose[*] then
cache.h would contain something like
#define MERGE_HEAD_FILE "MERGE_HEAD"
#define PREPARED_COMMIT_MESSAGE_FILE "MERGE_MSG"
#define <description of SQUASH_MSG goes here> "SQUASH_MSG"
and commit.c or cache.h would contain something like
#define EDITABLE_COMMIT_MESSAGE_FILE "COMMIT_EDITMSG"
and those symbolic names would be used throughout builtin code to
name those files. So we would get
(1) documentation of what filenames are special to git, collected
in one place
(2) protection against typos
without losing
(3) names that look like constants
Jonathan
[*] and I shouldn't! What I like should matter much less than what is
right.