* [PATCH 0/3] make commit --verbose work with --no-status @ 2014-02-21 19:09 Tay Ray Chuan 2014-02-21 19:09 ` [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT Tay Ray Chuan 0 siblings, 1 reply; 8+ messages in thread From: Tay Ray Chuan @ 2014-02-21 19:09 UTC (permalink / raw) To: Git Mailing List; +Cc: Tay Ray Chuan One would expect 'git commit --verbose --no-status' to give a commit message with a diff of the commit, sans the output of git-status. However, this does not work currently; the commit message body is entirely empty (diff is absent as well). This patch series attempts to make this work, as one would expect. [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT We first rename STATUS_FORMAT_NONE to *_DEFAULT, so that we can use it to mean "no status output at all". [PATCH 2/3] extract setting of wt_status.commitable flag out of wt_status_print_updated() Currently, the wt_status.commitable flag is set only when we call wt_status_print(). * Extract this logic, so that we don't have to go through the git-status output code. * In fact, it is not set when --short or --porcelain are used. This series does not attempt to fix the bug, though it should be trivial to do so with this patch. [PATCH 3/3] make commit --verbose work with --no-status Actual work here. -- 1.9.0.291.g027825b ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT 2014-02-21 19:09 [PATCH 0/3] make commit --verbose work with --no-status Tay Ray Chuan @ 2014-02-21 19:09 ` Tay Ray Chuan 2014-02-21 19:09 ` [PATCH 2/3] extract setting of wt_status.commitable flag out of wt_status_print_updated() Tay Ray Chuan 2014-02-22 8:24 ` [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT Jeff King 0 siblings, 2 replies; 8+ messages in thread From: Tay Ray Chuan @ 2014-02-21 19:09 UTC (permalink / raw) To: Git Mailing List; +Cc: Tay Ray Chuan, Jeff King Cc: Jeff King <peff@peff.net> In f3f47a1 (status: add --long output format option), STATUS_FORMAT_NONE was introduced, meaning "the user did not specify anything". Rename this to *_DEFAULT to better indicate its meaning. This paves the way for _NONE to really mean "no status". Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- builtin/commit.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 3783bca..2e86b76 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -125,7 +125,7 @@ static const char *only_include_assumed; static struct strbuf message = STRBUF_INIT; static enum status_format { - STATUS_FORMAT_NONE = 0, + STATUS_FORMAT_DEFAULT = 0, STATUS_FORMAT_LONG, STATUS_FORMAT_SHORT, STATUS_FORMAT_PORCELAIN, @@ -487,7 +487,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int case STATUS_FORMAT_UNSPECIFIED: die("BUG: finalize_deferred_config() should have been called"); break; - case STATUS_FORMAT_NONE: + case STATUS_FORMAT_DEFAULT: case STATUS_FORMAT_LONG: wt_status_print(s); break; @@ -1028,7 +1028,7 @@ static void finalize_deferred_config(struct wt_status *s) !s->null_termination); if (s->null_termination) { - if (status_format == STATUS_FORMAT_NONE || + if (status_format == STATUS_FORMAT_DEFAULT || status_format == STATUS_FORMAT_UNSPECIFIED) status_format = STATUS_FORMAT_PORCELAIN; else if (status_format == STATUS_FORMAT_LONG) @@ -1038,7 +1038,7 @@ static void finalize_deferred_config(struct wt_status *s) if (use_deferred_config && status_format == STATUS_FORMAT_UNSPECIFIED) status_format = status_deferred_config.status_format; if (status_format == STATUS_FORMAT_UNSPECIFIED) - status_format = STATUS_FORMAT_NONE; + status_format = STATUS_FORMAT_DEFAULT; if (use_deferred_config && s->show_branch < 0) s->show_branch = status_deferred_config.show_branch; @@ -1141,7 +1141,7 @@ static int parse_and_validate_options(int argc, const char *argv[], if (all && argc > 0) die(_("Paths with -a does not make sense.")); - if (status_format != STATUS_FORMAT_NONE) + if (status_format != STATUS_FORMAT_DEFAULT) dry_run = 1; return argc; @@ -1197,7 +1197,7 @@ static int git_status_config(const char *k, const char *v, void *cb) if (git_config_bool(k, v)) status_deferred_config.status_format = STATUS_FORMAT_SHORT; else - status_deferred_config.status_format = STATUS_FORMAT_NONE; + status_deferred_config.status_format = STATUS_FORMAT_DEFAULT; return 0; } if (!strcmp(k, "status.branch")) { @@ -1314,7 +1314,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) case STATUS_FORMAT_UNSPECIFIED: die("BUG: finalize_deferred_config() should have been called"); break; - case STATUS_FORMAT_NONE: + case STATUS_FORMAT_DEFAULT: case STATUS_FORMAT_LONG: s.verbose = verbose; s.ignore_submodule_arg = ignore_submodule_arg; @@ -1522,7 +1522,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) usage_with_options(builtin_commit_usage, builtin_commit_options); status_init_config(&s, git_commit_config); - status_format = STATUS_FORMAT_NONE; /* Ignore status.short */ + status_format = STATUS_FORMAT_DEFAULT; /* Ignore status.short */ s.colopts = 0; if (get_sha1("HEAD", sha1)) -- 1.9.0.291.g027825b ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] extract setting of wt_status.commitable flag out of wt_status_print_updated() 2014-02-21 19:09 ` [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT Tay Ray Chuan @ 2014-02-21 19:09 ` Tay Ray Chuan 2014-02-21 19:09 ` [PATCH 3/3] make commit --verbose work with --no-status Tay Ray Chuan 2014-02-22 8:24 ` [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT Jeff King 1 sibling, 1 reply; 8+ messages in thread From: Tay Ray Chuan @ 2014-02-21 19:09 UTC (permalink / raw) To: Git Mailing List; +Cc: Tay Ray Chuan At first glance, wt_status_print_updated() appears to be doing a bunch of printing (as its name implies), and it may not be immediately obvious that it also sets the vital wt_status.commitable flag. Extract this out into a separate function; it is hoped that the improved clarity to future Git contributors would outweigh the performance penalty. Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- wt-status.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index a452407..9b0189c 100644 --- a/wt-status.c +++ b/wt-status.c @@ -589,6 +589,21 @@ void wt_status_collect(struct wt_status *s) wt_status_collect_untracked(s); } +void wt_status_mark_commitable(struct wt_status *s) +{ + int i; + + for (i = 0; i < s->change.nr; i++) { + struct wt_status_change_data *d; + d = s->change.items[i].util; + if (!d->index_status || + d->index_status == DIFF_STATUS_UNMERGED) + continue; + s->commitable = 1; + break; + } +} + static void wt_status_print_unmerged(struct wt_status *s) { int shown_header = 0; @@ -627,7 +642,6 @@ static void wt_status_print_updated(struct wt_status *s) continue; if (!shown_header) { wt_status_print_cached_header(s); - s->commitable = 1; shown_header = 1; } wt_status_print_change_data(s, WT_STATUS_UPDATED, it); @@ -1309,6 +1323,7 @@ void wt_status_print(struct wt_status *s) status_printf_ln(s, color(WT_STATUS_HEADER, s), ""); } + wt_status_mark_commitable(s); wt_status_print_updated(s); wt_status_print_unmerged(s); wt_status_print_changed(s); -- 1.9.0.291.g027825b ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] make commit --verbose work with --no-status 2014-02-21 19:09 ` [PATCH 2/3] extract setting of wt_status.commitable flag out of wt_status_print_updated() Tay Ray Chuan @ 2014-02-21 19:09 ` Tay Ray Chuan 2014-02-22 8:31 ` Jeff King 0 siblings, 1 reply; 8+ messages in thread From: Tay Ray Chuan @ 2014-02-21 19:09 UTC (permalink / raw) To: Git Mailing List; +Cc: Tay Ray Chuan One would expect 'git commit --verbose --no-status' to give a commit message with a diff of the commit, sans the output of git-status. However, this does not work currently; the commit message body is entirely empty (diff is absent as well). This is because internally the status machinery is used to provide both the diff and status output, but it is skipped due to --no-status. We introduce a new status_format, STATUS_FORMAT_NONE. Thus, we still call run_status(), but produce nothing in place of the usual git-status output. status_format is set only when git-commit is passed i) --verbose, and ii) --no-status. Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- builtin/commit.c | 14 +++++++++++++- wt-status.c | 2 +- wt-status.h | 3 +++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 2e86b76..fca6a6b 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -126,6 +126,7 @@ static struct strbuf message = STRBUF_INIT; static enum status_format { STATUS_FORMAT_DEFAULT = 0, + STATUS_FORMAT_NONE, STATUS_FORMAT_LONG, STATUS_FORMAT_SHORT, STATUS_FORMAT_PORCELAIN, @@ -478,6 +479,10 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int wt_status_collect(s); switch (status_format) { + case STATUS_FORMAT_NONE: + wt_status_mark_commitable(s); + wt_status_print_verbose(s); + break; case STATUS_FORMAT_SHORT: wt_shortstatus_print(s); break; @@ -1141,7 +1146,12 @@ static int parse_and_validate_options(int argc, const char *argv[], if (all && argc > 0) die(_("Paths with -a does not make sense.")); - if (status_format != STATUS_FORMAT_DEFAULT) + if (verbose && !include_status) { + include_status = 1; + status_format = STATUS_FORMAT_NONE; + } + + if (status_format != STATUS_FORMAT_DEFAULT && !verbose) dry_run = 1; return argc; @@ -1305,6 +1315,8 @@ int cmd_status(int argc, const char **argv, const char *prefix) s.prefix = prefix; switch (status_format) { + case STATUS_FORMAT_NONE: + break; case STATUS_FORMAT_SHORT: wt_shortstatus_print(&s); break; diff --git a/wt-status.c b/wt-status.c index 9b0189c..e90fd0f 100644 --- a/wt-status.c +++ b/wt-status.c @@ -822,7 +822,7 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf) strbuf_release(&pattern); } -static void wt_status_print_verbose(struct wt_status *s) +void wt_status_print_verbose(struct wt_status *s) { struct rev_info rev; struct setup_revision_opt opt; diff --git a/wt-status.h b/wt-status.h index 30a4812..5c89f23 100644 --- a/wt-status.h +++ b/wt-status.h @@ -95,8 +95,11 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *); void wt_status_prepare(struct wt_status *s); void wt_status_print(struct wt_status *s); void wt_status_collect(struct wt_status *s); +void wt_status_mark_commitable(struct wt_status *state); void wt_status_get_state(struct wt_status_state *state, int get_detached_from); +void wt_status_print_verbose(struct wt_status *s); + void wt_shortstatus_print(struct wt_status *s); void wt_porcelain_print(struct wt_status *s); -- 1.9.0.291.g027825b ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] make commit --verbose work with --no-status 2014-02-21 19:09 ` [PATCH 3/3] make commit --verbose work with --no-status Tay Ray Chuan @ 2014-02-22 8:31 ` Jeff King 2014-02-24 4:24 ` Tay Ray Chuan 0 siblings, 1 reply; 8+ messages in thread From: Jeff King @ 2014-02-22 8:31 UTC (permalink / raw) To: Tay Ray Chuan; +Cc: Git Mailing List On Sat, Feb 22, 2014 at 03:09:22AM +0800, Tay Ray Chuan wrote: > @@ -1141,7 +1146,12 @@ static int parse_and_validate_options(int argc, const char *argv[], > if (all && argc > 0) > die(_("Paths with -a does not make sense.")); > > - if (status_format != STATUS_FORMAT_DEFAULT) > + if (verbose && !include_status) { > + include_status = 1; > + status_format = STATUS_FORMAT_NONE; > + } > + > + if (status_format != STATUS_FORMAT_DEFAULT && !verbose) > dry_run = 1; What happens here when there is an alternate status format _and_ --verbose is used? If I say "git commit --porcelain" it should imply --dry-run. But "git commit --porcelain --verbose" no longer does so after your patch. -Peff ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] make commit --verbose work with --no-status 2014-02-22 8:31 ` Jeff King @ 2014-02-24 4:24 ` Tay Ray Chuan 2014-02-24 8:33 ` Jeff King 0 siblings, 1 reply; 8+ messages in thread From: Tay Ray Chuan @ 2014-02-24 4:24 UTC (permalink / raw) To: Jeff King; +Cc: Git Mailing List On Sat, Feb 22, 2014 at 4:31 PM, Jeff King <peff@peff.net> wrote: > On Sat, Feb 22, 2014 at 03:09:22AM +0800, Tay Ray Chuan wrote: > >> @@ -1141,7 +1146,12 @@ static int parse_and_validate_options(int argc, const char *argv[], >> if (all && argc > 0) >> die(_("Paths with -a does not make sense.")); >> >> - if (status_format != STATUS_FORMAT_DEFAULT) >> + if (verbose && !include_status) { >> + include_status = 1; >> + status_format = STATUS_FORMAT_NONE; >> + } >> + >> + if (status_format != STATUS_FORMAT_DEFAULT && !verbose) >> dry_run = 1; > > What happens here when there is an alternate status format _and_ > --verbose is used? If I say "git commit --porcelain" it should imply > --dry-run. But "git commit --porcelain --verbose" no longer does so > after your patch. I should have just left the --dry-run inference alone, like this. -->8-- @@ -1141,7 +1146,12 @@ static int parse_and_validate_options if (all && argc > 0) die(_("Paths with -a does not make sense.")); - if (status_format != STATUS_FORMAT_DEFAULT) + if (verbose && !include_status) { + include_status = 1; + status_format = STATUS_FORMAT_NONE; + } else if (status_format != STATUS_FORMAT_DEFAULT) dry_run = 1; return argc; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] make commit --verbose work with --no-status 2014-02-24 4:24 ` Tay Ray Chuan @ 2014-02-24 8:33 ` Jeff King 0 siblings, 0 replies; 8+ messages in thread From: Jeff King @ 2014-02-24 8:33 UTC (permalink / raw) To: Tay Ray Chuan; +Cc: Git Mailing List On Mon, Feb 24, 2014 at 12:24:42PM +0800, Tay Ray Chuan wrote: > > What happens here when there is an alternate status format _and_ > > --verbose is used? If I say "git commit --porcelain" it should imply > > --dry-run. But "git commit --porcelain --verbose" no longer does so > > after your patch. > > I should have just left the --dry-run inference alone, like this. > > -->8-- > > @@ -1141,7 +1146,12 @@ static int parse_and_validate_options > if (all && argc > 0) > die(_("Paths with -a does not make sense.")); > > - if (status_format != STATUS_FORMAT_DEFAULT) > + if (verbose && !include_status) { > + include_status = 1; > + status_format = STATUS_FORMAT_NONE; > + } else if (status_format != STATUS_FORMAT_DEFAULT) > dry_run = 1; > > return argc; Hrm, not quite, because the way include_status works is weird. If I turn it off in the config, like this: git config commit.status false then asking explicitly for a status format should still dry-run and show it: git commit --porcelain IOW, include_status is only respected when we are generating the actual commit. So I think you need something more like: if (status_format == STATUS_FORMAT_DEFAULT) { if (verbose && !include_status) { include_status = 1; status_format = STATUS_FORMAT_NONE; } } else dry_run = 1; -Peff ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT 2014-02-21 19:09 ` [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT Tay Ray Chuan 2014-02-21 19:09 ` [PATCH 2/3] extract setting of wt_status.commitable flag out of wt_status_print_updated() Tay Ray Chuan @ 2014-02-22 8:24 ` Jeff King 1 sibling, 0 replies; 8+ messages in thread From: Jeff King @ 2014-02-22 8:24 UTC (permalink / raw) To: Tay Ray Chuan; +Cc: Git Mailing List On Sat, Feb 22, 2014 at 03:09:20AM +0800, Tay Ray Chuan wrote: > In f3f47a1 (status: add --long output format option), STATUS_FORMAT_NONE > was introduced, meaning "the user did not specify anything". Rename this > to *_DEFAULT to better indicate its meaning. Hmm. We later introduced STATUS_FORMAT_UNSPECIFIED in 84b4202d. It seems like that is the same thing as the _DEFAULT you are proposing here. Can we collapse them into a single value? -Peff ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-02-24 8:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-21 19:09 [PATCH 0/3] make commit --verbose work with --no-status Tay Ray Chuan 2014-02-21 19:09 ` [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT Tay Ray Chuan 2014-02-21 19:09 ` [PATCH 2/3] extract setting of wt_status.commitable flag out of wt_status_print_updated() Tay Ray Chuan 2014-02-21 19:09 ` [PATCH 3/3] make commit --verbose work with --no-status Tay Ray Chuan 2014-02-22 8:31 ` Jeff King 2014-02-24 4:24 ` Tay Ray Chuan 2014-02-24 8:33 ` Jeff King 2014-02-22 8:24 ` [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).