status -s: Use the same config as status

This mini series is an RFC for bringing the short version of status in line with the longer one. They already both obey status.showUntrackedFiles. Currently, status -s does not obey status.relativePaths nor color.status.

I think status should follow the priniciple of least surprise here in the sense that -s should not change other aspects of the output (although it is very different internally).

I think we don't have any tests for status -s yet, which is one reason why this is an RFC: I don't expect to get through with it, like the introducer of status -s did ;) Another reason is mentioned in 2/2.

Michael J Gruber (2):
status -s: respect the status.relativePaths option status -s: obey color.status

 builtin-commit.c |   22 +++++++++++++++++-----
 wt-status.c      |    2 +-
 wt-status.h      |    1 +
 3 files changed, 19 insertions(+), 6 deletions(-)

so that 'status' and 'status -s' in a subdir produce the same file names.

---
Anything else would be highly surprising. Note that the porcelain case is unchanged, of course.

 builtin-commit.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

index 7f888fe..c103beb 100644
@@ -1113,6 +1113,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	switch (status_format) {
 	case STATUS_FORMAT_SHORT:
+		if (s.relative_paths)
+			s.prefix = prefix;
 		short_print(&s, null_termination);
 		break;
 	case STATUS_FORMAT_PORCELAIN:

1.6.6.rc0.274.g71380

Thanks, I had been meaning to look into this (who knew it would only be two lines ;) ).

Now that short and porcelain formats are diverging a bit, we should probably have a few tests. I think there may already be a bug in

  git commit --dry-run --porcelain

as it looks like commit will use color and relative path configuration even for the porcelain version.

-Peff

This configuration is on by default which makes this change even more important.

I'd squash this in and queue it at the tip of jk/1.7.0-status topic.

Thanks

index 58d35fb..b3dfa42 100644
@@ -114,8 +114,8 @@ compatibility) and `color.status.<slot>` configuration variables
 to colorize its output.
 
 If the config variable `status.relativePaths` is set to false, then all
-paths shown in the long format are relative to the repository root, not
-to the current directory.
+paths shown are relative to the repository root, not to the current
+directory.
 
 If `status.submodulesummary` is set to a non zero number or true (identical
 to -1 or an unlimited number), the submodule summary will be enabled for

Make the short version of status obey the color.status boolean. We color the status letters only, because they carry the state information and are potentially colored differently, such as for a file with staged changes as well as changes in the worktree against the index.

---
This is RFC for several reasons:

* Should I rename wt-status.c's color() into something more unique when I export it?
* Is there any policy regarding use of putchar/puts vs. printf?
* The way it is done now I "color" a space, otherwise one would need to break down the print statements even more. Since we always color the foreground only it is no problem, is it?
* Even if I were to write tests for status -s: How do I test colors?

 builtin-commit.c |   20 +++++++++++++++-----
 wt-status.c      |    2 +-
 wt-status.h      |    1 +
 3 files changed, 17 insertions(+), 6 deletions(-)

index c103beb..c38fc96 100644
@@ -999,7 +999,7 @@ static void short_unmerged(int null_termination, struct string_list_item *it,
 	case 6: how = "AA"; break; /* both added */
 	case 7: how = "UU"; break; /* both modified */
 	}
-	printf("%s ", how);
+	color_fprintf(s->fp, color(WT_STATUS_UNMERGED, s), "%s ", how);
 	if (null_termination) {
 		fprintf(stdout, "%s%c", it->string, 0);
 	} else {

@@ -1016,9 +1016,14 @@ static void short_status(int null_termination, struct string_list_item *it,
 {
 	struct wt_status_change_data *d = it->util;
 
-	printf("%c%c ",
-	       !d->index_status ? ' ' : d->index_status,
-	       !d->worktree_status ? ' ' : d->worktree_status);
+	if (d->index_status)
+		color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%c", d->index_status);
+	else
+		putchar(' ');
+	if (d->worktree_status)
+		color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%c ", d->worktree_status);
+	else
+		printf("  ");
 	if (null_termination) {
 		fprintf(stdout, "%s%c", it->string, 0);
 		if (d->head_path)

@@ -1046,7 +1051,8 @@ static void short_untracked(int null_termination, struct string_list_item *it,
 		struct strbuf onebuf = STRBUF_INIT;
 		const char *one;
 		one = quote_path(it->string, -1, &onebuf, s->prefix);
-		printf("?? %s\n", one);
+		color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "?? ");
+		puts(one);
 		strbuf_release(&onebuf);
 	}
 }

@@ -1115,6 +1121,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	case STATUS_FORMAT_SHORT:
 		if (s.relative_paths)
 			s.prefix = prefix;
+		if (s.use_color == -1)
+			s.use_color = git_use_color_default;
+		if (diff_use_color_default == -1)
+			diff_use_color_default = git_use_color_default;
 		short_print(&s, null_termination);
 		break;
 	case STATUS_FORMAT_PORCELAIN:

index 3c2f580..c779b7c 100644
@@ -19,7 +19,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RED,    /* WT_STATUS_UNMERGED */
 };
 
-static const char *color(int slot, struct wt_status *s)
+const char *color(int slot, struct wt_status *s)
 {
 	return s->use_color > 0 ? s->color_palette[slot] : "";
 }

index 09fd9f1..22b15b0 100644
@@ -55,5 +55,6 @@ struct wt_status {
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
+const char *color(int slot, struct wt_status *s);
 
 #endif /* STATUS_H */

1.6.6.rc0.274.g71380

If the printed string contains color escapes that should be obeyed, you can use only fputs, printf, and fprintf. You should not use puts or putchar.

See t4034-diff-words.sh.

-- Hannes

Johannes Sixt venit, vidit, dixit 26.11.2009 16:36:

Oh, I don't use them with colors. I use only color_fprintf for that. I was wondering whether there's a preference of printf("%c", c) over putchar(c), for example.

Thanks, I'll keep that in mind for the case that I do have to write test ;)

Michael

This msysgit-imposed restriction is something even I do not remember offhand. Could you please document it somewhere in a file any developer would get by checking out the 'master' branch of git.git?

From: Johannes Sixt <[email protected]>

We emulate color escape codes on Windows by overriding printf, fprintf, and fputs. Warn users that these are the only functions that can be used to print them.

---

Like this?

 color.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

index 7d8da6f..edeaa3e 100644
@@ -4,6 +4,11 @@
 /* "\033[1;38;5;2xx;48;5;2xxm\0" is 23 bytes */
 #define COLOR_MAXLEN 24
 
+/*
+ * IMPORTANT: Due to the way these color codes are emulated on Windows,
+ * write them only using printf, fprintf, and fputs. In particular,
+ * do not use puts.
+ */
 #define GIT_COLOR_NORMAL	""
 #define GIT_COLOR_RESET		"\033[m"
 #define GIT_COLOR_BOLD		"\033[1m"

1.6.6.rc0.43.g50037

Yeah, and in addition to "puts", "write(2)" is also not supported, right?

Correct, good catch!

--- 8< ---
From: Johannes Sixt <[email protected]>
Subject: [PATCH] Add a notice that only certain functions can print color escape codes

We emulate color escape codes on Windows by overriding printf, fprintf, and fputs. Warn users that these are the only functions that can be used to print them.

---
 color.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

index 7d8da6f..3cb4b7f 100644
@@ -4,6 +4,11 @@
 /* "\033[1;38;5;2xx;48;5;2xxm\0" is 23 bytes */
 #define COLOR_MAXLEN 24
 
+/*
+ * IMPORTANT: Due to the way these color codes are emulated on Windows,
+ * write them only using printf(), fprintf(), and fputs(). In particular,
+ * do not use puts() or write().
+ */
 #define GIT_COLOR_NORMAL	""
 #define GIT_COLOR_RESET		"\033[m"
 #define GIT_COLOR_BOLD		"\033[1m"

1.6.6.rc0.43.g50037

Johannes Sixt venit, vidit, dixit 27.11.2009 08:42:

Is it an option to instead move short_unmerged(), short_status() and friends to wt-status.c from builtin-commit.c? It's been quite a while since I worked on the code, so I don't recall why it needs such cross references at low level between two files.

J6t addressed it. You have mixture of putchar(' ') and printf(" ") which looks somewhat funny ;-)

Some people do configure to use "reverse". For example, I have:

[diff.color]
old = red reverse
whitespace = blue reverse

[status.color]
updated = green
changed = red
untracked = blue reverse

The output should be consistent between long and short format (I do not offhand recall what we do for the long format, though).

Junio C Hamano venit, vidit, dixit 27.11.2009 06:15:

I didn't notice, but I'll look into it during the rewrite.

;) I'm happy with printf("c"), I just thought someone would find printf overkill for a format less char.

Oh, I didn't know about reverse. In this case I have to change the code and leave the space alone. (The one between ?? and the filename.) Will do.

In the long format, only the file name is colored. Note that in the short format, it does not make sense to color the file name because one line may represent two pieces of status information. That is why I color the two status letters and not the file name.

Michael