[RFC PATCH 0/5] Introduce -t, --table for status/add commands

[RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

This is a proposal / proof-of-concept for a new table-based output
format for the git status command, and for dry runs (-n) of the git add
command. This could be extended to create visual dry runs for other
other commands like rm, mv, restore, stash, commit, and clean.

For some context, earlier this year I released a tool called Git-Sim
(https://github.com/initialcommit-com/git-sim) which allows users to do
visual dry runs of many Git commands, which are rendered as high quality
output image files. Simulating commands like status, add, rm, mv, restore,
stash, and commit creates a table with 3 columns to represent the way file
changes "move around" as a result of the command being simulated.

I've gotten positive feedback from users about this visual approach to
simulating git commands, which is more intuitive than pure terminal text
for both newer users to understand how git works and for visual people.

As a result, I was thinking of ways to integrate these types of visual
formats directly into Git. A table-based output format with colored
highlighting for the commands mentioned above is low hanging fruit.

Teach 'git status' the new -t, --table flag, which displays the status
output in a 3-column table format, preserving terminology and color
coding from the default git status "long output" format (note that the
column headers are shortened here for the small width of this email, and
also I just realized that the tables below might not look right on the
mailing list due to the differing character width, but it looks correct
in the terminal so please test there it's more fun anyway :D):

$ git status -t
-------------------------------------------------------------------------
|    Untracked files    | Changes n...or commit | Changes t...committed |
-------------------------------------------------------------------------
|         poiu          |                       |                       |
|     status-table/     |                       |                       |
|                       |                       |         asdf          |
|                       |        table.c        |                       |
|                       |      wt-status.c      |                       |
-------------------------------------------------------------------------

Teach 'git add' the new -t, --table flag to be used ONLY in combination
with the '-n' flag for dry runs. Instead of simply printing out the
added filenames, the full status table format is displayed, along with
arrows that visually show how the added files are being moved around:

$ git add -nt poiu wt-status.c
-------------------------------------------------------------------------
|    Untracked files    | Changes n...or commit | Changes t...committed |
-------------------------------------------------------------------------
|         poiu -----------------------------------------> poiu          |
|     status-table/     |                       |                       |
|                       |                       |         asdf          |
|                       |        table.c        |                       |
|                       |      wt-status.c ----------> wt-status.c      |
-------------------------------------------------------------------------

Other notes:

* The width of the table and columns are dynamically set based on the
  width of the terminal.

* Long paths are shortened to include the maximum number of characters
  from both ends of the path that will fit, with a '...' in the middle.

* Color coding matches the default output of 'git status', with
  untracked files and working dir mods in red, and staged changes in
  green. If needed, arrows are drawn in cyan.

As stated above, the dry run version of the table format can be applied
to various other commands like rm, mv, restore, stash, commit, and clean
which all move file changes around in a way that can be represented in
the table format. New columns may need to be added or arrows reversed
to show changes moving in various directions. Note that some of these
commands don't appear to have a dry run (-n) option yet, so it could be
added for consistency (if not already in use) and for use with the new
table format.

Since this is an RFC patch series, I probably did some illegal and dumb
things in my code changes just to get it into a demo-able state. I am a
bit wary of having made changes to files like "read-cache.c" and
"read-cache-ll.h" to pass in the wt_status info, and there are probably
betters ways to do some other things too.

Feedback on both the new format itself and the implementation is very
much appreciated!

Jacob Stopak (5):
  status: introduce -t, --table flag
  status: handle long paths with -t, --table flag
  status: add advice arg for -t, --table flag
  add: add -t, --table flag for visual dry runs
  add: set unique color for -t, --table arrows

 Makefile         |   1 +
 builtin/add.c    |  46 +++++++--
 builtin/commit.c |   4 +-
 read-cache-ll.h  |   9 +-
 read-cache.c     |  32 ++++++-
 table.c          | 245 +++++++++++++++++++++++++++++++++++++++++++++++
 table.h          |   6 ++
 wt-status.c      |  74 +++++++++-----
 wt-status.h      |   3 +
 9 files changed, 378 insertions(+), 42 deletions(-)
 create mode 100644 table.c
 create mode 100644 table.h

-- 
2.42.0.402.gbe8243af7b.dirty

[RFC PATCH 1/5] status: introduce -t, --table flag

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 Makefile         |   1 +
 builtin/commit.c |   2 +
 table.c          | 117 +++++++++++++++++++++++++++++++++++++++++++++++
 table.h          |   6 +++
 wt-status.c      |  72 +++++++++++++++++++----------
 wt-status.h      |   1 +
 6 files changed, 174 insertions(+), 25 deletions(-)
 create mode 100644 table.c
 create mode 100644 table.h

diff --git a/Makefile b/Makefile
index 9c6a2f125f..a7399ca8f0 100644
--- a/Makefile
+++ b/Makefile
@@ -1155,6 +1155,7 @@ LIB_OBJS += submodule-config.o
 LIB_OBJS += submodule.o
 LIB_OBJS += symlinks.o
 LIB_OBJS += tag.o
+LIB_OBJS += table.o
 LIB_OBJS += tempfile.o
 LIB_OBJS += thread-utils.o
 LIB_OBJS += tmp-objdir.o
diff --git a/builtin/commit.c b/builtin/commit.c
index 7da5f92448..4338896dbf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1539,6 +1539,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		OPT_CALLBACK_F('M', "find-renames", &rename_score_arg,
 		  N_("n"), N_("detect renames, optionally set similarity index"),
 		  PARSE_OPT_OPTARG | PARSE_OPT_NONEG, opt_parse_rename_score),
+		OPT_SET_INT('t', "table", &status_format,
+			    N_("show status in table format"), STATUS_FORMAT_TABLE),
 		OPT_END(),
 	};
 
diff --git a/table.c b/table.c
new file mode 100644
index 0000000000..54cf9e4d07
--- /dev/null
+++ b/table.c
@@ -0,0 +1,117 @@
+#define USE_THE_INDEX_VARIABLE
+#include "builtin.h"
+#include "gettext.h"
+#include "strbuf.h"
+#include "wt-status.h"
+#include "config.h"
+#include "string-list.h"
+#include "sys/ioctl.h"
+
+static const char *color(int slot, struct wt_status *s)
+{
+	const char *c = "";
+	if (want_color(s->use_color))
+		c = s->color_palette[slot];
+	if (slot == WT_STATUS_ONBRANCH && color_is_nil(c))
+		c = s->color_palette[WT_STATUS_HEADER];
+	return c;
+}
+
+static void build_table_border(struct strbuf *buf, int cols)
+{
+	strbuf_reset(buf);
+	strbuf_addchars(buf, '-', cols);
+}
+
+static void build_table_entry(struct strbuf *buf, char *entry, int cols)
+{
+	strbuf_reset(buf);
+	strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+	strbuf_addstr(buf, entry);
+
+	/* Bump right padding if entry length is odd */
+	if (!(strlen(entry) % 2))
+		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2 + 1);
+	else
+		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+}
+
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+{
+	printf(_("|"));
+	color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
+	printf(_("|"));
+	color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
+	printf(_("|"));
+	color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
+	printf(_("|\n"));
+}
+
+void build_and_draw_status_table(struct wt_status *s)
+{
+	struct winsize w;
+	int cols;
+	struct strbuf table_border = STRBUF_INIT;
+	struct strbuf table_col_entry_1 = STRBUF_INIT;
+	struct strbuf table_col_entry_2 = STRBUF_INIT;
+	struct strbuf table_col_entry_3 = STRBUF_INIT;
+	struct string_list_item *item;
+
+	/* Get terminal width */
+	ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
+	cols = w.ws_col;
+
+	/* Ensure table is divisible into 3 even columns */
+	while (((cols - 1) % 3) > 0 || !(cols % 2)) {
+		cols -= 1;
+	}
+
+	build_table_border(&table_border, cols);
+	build_table_entry(&table_col_entry_1, "Untracked files", cols);
+	build_table_entry(&table_col_entry_2, "Changes not staged for commit", cols);
+	build_table_entry(&table_col_entry_3, "Changes to be committed", cols);
+
+	/* Draw table header */
+	printf(_("%s\n"), table_border.buf);
+	printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+	printf(_("%s\n"), table_border.buf);
+
+	/* Draw table body */
+	for_each_string_list_item(item, &s->untracked) {
+		build_table_entry(&table_col_entry_1, item->string, cols);
+		build_table_entry(&table_col_entry_2, "", cols);
+		build_table_entry(&table_col_entry_3, "", cols);
+		print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+	}
+
+	for_each_string_list_item(item, &s->change) {
+		struct wt_status_change_data *d = item->util;
+		if (d->worktree_status && d->index_status) {
+			build_table_entry(&table_col_entry_1, "", cols);
+			build_table_entry(&table_col_entry_2, item->string, cols);
+			build_table_entry(&table_col_entry_3, item->string, cols);
+		} else if (d->worktree_status) {
+			build_table_entry(&table_col_entry_1, "", cols);
+			build_table_entry(&table_col_entry_2, item->string, cols);
+			build_table_entry(&table_col_entry_3, "", cols);
+		} else if (d->index_status) {
+			build_table_entry(&table_col_entry_1, "", cols);
+			build_table_entry(&table_col_entry_2, "", cols);
+			build_table_entry(&table_col_entry_3, item->string, cols);
+		}
+		print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+	}
+	
+	if (!s->untracked.nr && !s->change.nr) {
+		build_table_entry(&table_col_entry_1, "-", cols);
+		build_table_entry(&table_col_entry_2, "-", cols);
+		build_table_entry(&table_col_entry_3, "-", cols);
+		printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+	}
+
+	printf(_("%s\n"), table_border.buf);
+	strbuf_release(&table_border);
+	strbuf_release(&table_col_entry_1);
+	strbuf_release(&table_col_entry_2);
+	strbuf_release(&table_col_entry_3);
+}
diff --git a/table.h b/table.h
new file mode 100644
index 0000000000..30e0d5509b
--- /dev/null
+++ b/table.h
@@ -0,0 +1,6 @@
+#ifndef TABLE_H
+#define TABLE_H
+
+void build_and_draw_status_table(struct wt_status *s);
+
+#endif /* TABLE_H */
diff --git a/wt-status.c b/wt-status.c
index 9f45bf6949..24b56ea559 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -31,6 +31,7 @@
 #include "lockfile.h"
 #include "sequencer.h"
 #include "fsmonitor-settings.h"
+#include "table.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
 #define UF_DELAY_WARNING_IN_MS (2 * 1000)
@@ -1833,39 +1834,46 @@ static void wt_longstatus_print_state(struct wt_status *s)
 		show_sparse_checkout_in_use(s, state_color);
 }
 
-static void wt_longstatus_print(struct wt_status *s)
+static void wt_longstatus_print_onwhat(struct wt_status *s, const char *branch_name)
 {
+	const char *on_what = _("On branch ");
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+
+	if (!strcmp(branch_name, "HEAD")) {
+		branch_status_color = color(WT_STATUS_NOBRANCH, s);
+		if (s->state.rebase_in_progress ||
+		    s->state.rebase_interactive_in_progress) {
+			if (s->state.rebase_interactive_in_progress)
+				on_what = _("interactive rebase in progress; onto ");
+			else
+				on_what = _("rebase in progress; onto ");
+			branch_name = s->state.onto;
+		} else if (s->state.detached_from) {
+			branch_name = s->state.detached_from;
+			if (s->state.detached_at)
+				on_what = _("HEAD detached at ");
+			else
+				on_what = _("HEAD detached from ");
+		} else {
+			branch_name = "";
+			on_what = _("Not currently on any branch.");
+		}
+	} else
+		skip_prefix(branch_name, "refs/heads/", &branch_name);
+
+	status_printf_more(s, branch_status_color, "%s", on_what);
+	status_printf_more(s, branch_color, "%s\n", branch_name);
+}
+
+static void wt_longstatus_print(struct wt_status *s)
+{
 	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
-		const char *on_what = _("On branch ");
 		const char *branch_name = s->branch;
-		if (!strcmp(branch_name, "HEAD")) {
-			branch_status_color = color(WT_STATUS_NOBRANCH, s);
-			if (s->state.rebase_in_progress ||
-			    s->state.rebase_interactive_in_progress) {
-				if (s->state.rebase_interactive_in_progress)
-					on_what = _("interactive rebase in progress; onto ");
-				else
-					on_what = _("rebase in progress; onto ");
-				branch_name = s->state.onto;
-			} else if (s->state.detached_from) {
-				branch_name = s->state.detached_from;
-				if (s->state.detached_at)
-					on_what = _("HEAD detached at ");
-				else
-					on_what = _("HEAD detached from ");
-			} else {
-				branch_name = "";
-				on_what = _("Not currently on any branch.");
-			}
-		} else
-			skip_prefix(branch_name, "refs/heads/", &branch_name);
 		status_printf(s, color(WT_STATUS_HEADER, s), "%s", "");
-		status_printf_more(s, branch_status_color, "%s", on_what);
-		status_printf_more(s, branch_color, "%s\n", branch_name);
+		wt_longstatus_print_onwhat(s, branch_name);
 		if (!s->is_initial)
 			wt_longstatus_print_tracking(s);
 	}
@@ -2133,6 +2141,17 @@ static void wt_shortstatus_print(struct wt_status *s)
 		wt_shortstatus_other(it, s, "!!");
 }
 
+static void wt_tablestatus_print(struct wt_status *s)
+{
+	if (s->show_branch) {
+		const char *branch_name = s->branch;
+		wt_longstatus_print_onwhat(s, branch_name);
+		wt_longstatus_print_tracking(s);
+	}
+
+	build_and_draw_status_table(s);
+}
+
 static void wt_porcelain_print(struct wt_status *s)
 {
 	s->use_color = 0;
@@ -2560,6 +2579,9 @@ void wt_status_print(struct wt_status *s)
 	case STATUS_FORMAT_LONG:
 		wt_longstatus_print(s);
 		break;
+	case STATUS_FORMAT_TABLE:
+		wt_tablestatus_print(s);
+		break;
 	}
 
 	trace2_region_leave("status", "print", s->repo);
diff --git a/wt-status.h b/wt-status.h
index ab9cc9d8f0..70a3b7a2e4 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -73,6 +73,7 @@ enum wt_status_format {
 	STATUS_FORMAT_SHORT,
 	STATUS_FORMAT_PORCELAIN,
 	STATUS_FORMAT_PORCELAIN_V2,
+	STATUS_FORMAT_TABLE,
 
 	STATUS_FORMAT_UNSPECIFIED
 };
-- 
2.42.0.402.gbe8243af7b.dirty

[RFC PATCH 2/5] status: handle long paths with -t, --table flag

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 table.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/table.c b/table.c
index 54cf9e4d07..87b6df8c66 100644
--- a/table.c
+++ b/table.c
@@ -25,15 +25,44 @@ static void build_table_border(struct strbuf *buf, int cols)
 
 static void build_table_entry(struct strbuf *buf, char *entry, int cols)
 {
+	int len = strlen(entry);
+	size_t col_width = (cols / 3) - 5; /* subtract for padding */
+
 	strbuf_reset(buf);
-	strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+
+	/* Trim equally from both sides if it doesn't fit in column */
+	if (len > col_width) {
+		struct strbuf start_buf = STRBUF_INIT;
+		struct strbuf end_buf = STRBUF_INIT;
+		struct strbuf entry_buf = STRBUF_INIT;
+
+		strbuf_addstr(&start_buf, entry);
+		strbuf_addstr(&end_buf, entry);
+
+		strbuf_remove(&start_buf, col_width / 2, len - col_width / 2);
+		strbuf_remove(&end_buf, 0, len - col_width / 2);
+
+		strbuf_addstr(&entry_buf, start_buf.buf);
+		strbuf_addstr(&entry_buf, "...");
+		strbuf_addstr(&entry_buf, end_buf.buf);
+
+		entry = strbuf_detach(&entry_buf, &col_width);
+		len = strlen(entry);
+
+		strbuf_release(&start_buf);
+		strbuf_release(&end_buf);
+		strbuf_release(&entry_buf);
+	}
+
+	strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2); /* left padding */
 	strbuf_addstr(buf, entry);
 
-	/* Bump right padding if entry length is odd */
-	if (!(strlen(entry) % 2))
-		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2 + 1);
+	/* right padding */
+	if (!(len % 2))
+		/* Bump right padding if entry length is odd */
+		strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2 + 1);
 	else
-		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+		strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
 }
 
 static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
-- 
2.42.0.402.gbe8243af7b.dirty

[RFC PATCH 3/5] status: add advice arg for -t, --table flag

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 table.c     | 19 +++++++++++++++++--
 table.h     |  2 +-
 wt-status.c |  2 +-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/table.c b/table.c
index 87b6df8c66..73751339da 100644
--- a/table.c
+++ b/table.c
@@ -76,7 +76,7 @@ static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, stru
 	printf(_("|\n"));
 }
 
-void build_and_draw_status_table(struct wt_status *s)
+void build_and_draw_status_table(struct wt_status *s, int add_advice)
 {
 	struct winsize w;
 	int cols;
@@ -95,14 +95,29 @@ void build_and_draw_status_table(struct wt_status *s)
 		cols -= 1;
 	}
 
+	/* Draw table header */
 	build_table_border(&table_border, cols);
 	build_table_entry(&table_col_entry_1, "Untracked files", cols);
 	build_table_entry(&table_col_entry_2, "Changes not staged for commit", cols);
 	build_table_entry(&table_col_entry_3, "Changes to be committed", cols);
 
-	/* Draw table header */
 	printf(_("%s\n"), table_border.buf);
 	printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+
+	if (add_advice) {
+		build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
+		build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
+		build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
+
+		printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+
+		build_table_entry(&table_col_entry_1, "", cols);
+		build_table_entry(&table_col_entry_2, "(discard: git restore --staged <file>)", cols);
+		build_table_entry(&table_col_entry_3, "", cols);
+
+		printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+	}
+
 	printf(_("%s\n"), table_border.buf);
 
 	/* Draw table body */
diff --git a/table.h b/table.h
index 30e0d5509b..6017923bf9 100644
--- a/table.h
+++ b/table.h
@@ -1,6 +1,6 @@
 #ifndef TABLE_H
 #define TABLE_H
 
-void build_and_draw_status_table(struct wt_status *s);
+void build_and_draw_status_table(struct wt_status *s, int i);
 
 #endif /* TABLE_H */
diff --git a/wt-status.c b/wt-status.c
index 24b56ea559..62731859fe 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2149,7 +2149,7 @@ static void wt_tablestatus_print(struct wt_status *s)
 		wt_longstatus_print_tracking(s);
 	}
 
-	build_and_draw_status_table(s);
+	build_and_draw_status_table(s, 0);
 }
 
 static void wt_porcelain_print(struct wt_status *s)
-- 
2.42.0.402.gbe8243af7b.dirty

[RFC PATCH 4/5] add: add -t, --table flag for visual dry runs

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 builtin/add.c    | 46 ++++++++++++++++++------
 builtin/commit.c |  2 +-
 read-cache-ll.h  |  9 ++++-
 read-cache.c     | 32 ++++++++++++++---
 table.c          | 92 +++++++++++++++++++++++++++++++++++++++++++-----
 wt-status.c      |  1 +
 wt-status.h      |  1 +
 7 files changed, 157 insertions(+), 26 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index c27254a5cd..35ea1deda5 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -27,6 +27,7 @@
 #include "strvec.h"
 #include "submodule.h"
 #include "add-interactive.h"
+#include "wt-status.h"
 
 static const char * const builtin_add_usage[] = {
 	N_("git add [<options>] [--] <pathspec>..."),
@@ -221,7 +222,7 @@ N_("The following paths are ignored by one of your .gitignore files:\n");
 
 static int verbose, show_only, ignored_too, refresh_only;
 static int ignore_add_errors, intent_to_add, ignore_missing;
-static int warn_on_embedded_repo = 1;
+static int table_format, warn_on_embedded_repo = 1;
 
 #define ADDREMOVE_DEFAULT 1
 static int addremove = ADDREMOVE_DEFAULT;
@@ -264,6 +265,8 @@ static struct option builtin_add_options[] = {
 			N_("warn when adding an embedded repository")),
 	OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
 	OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+	OPT_SET_INT('t', "table", &table_format,
+		    N_("show status in table format"), STATUS_FORMAT_TABLE),
 	OPT_END(),
 };
 
@@ -322,7 +325,7 @@ static void check_embedded_repo(const char *path)
 	strbuf_release(&name);
 }
 
-static int add_files(struct dir_struct *dir, int flags)
+static int add_files(struct dir_struct *dir, int flags, struct wt_status *status)
 {
 	int i, exit_status = 0;
 	struct string_list matched_sparse_paths = STRING_LIST_INIT_NODUP;
@@ -345,7 +348,7 @@ static int add_files(struct dir_struct *dir, int flags)
 					   dir->entries[i]->name);
 			continue;
 		}
-		if (add_file_to_index(&the_index, dir->entries[i]->name, flags)) {
+		if (add_file_to_index_with_status(&the_index, dir->entries[i]->name, flags, status)) {
 			if (!ignore_add_errors)
 				die(_("adding files failed"));
 			exit_status = 1;
@@ -374,6 +377,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int require_pathspec;
 	char *seen = NULL;
 	struct lock_file lock_file = LOCK_INIT;
+	struct wt_status status;
+	unsigned int progress_flag = 0;
 
 	git_config(add_config, NULL);
 
@@ -459,7 +464,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		 (intent_to_add ? ADD_CACHE_INTENT : 0) |
 		 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
 		 (!(addremove || take_worktree_changes)
-		  ? ADD_CACHE_IGNORE_REMOVAL : 0));
+		  ? ADD_CACHE_IGNORE_REMOVAL : 0) |
+		 (table_format ? ADD_CACHE_FORMAT_TABLE : 0));
 
 	if (repo_read_index_preload(the_repository, &pathspec, 0) < 0)
 		die(_("index file corrupt"));
@@ -551,15 +557,35 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	begin_odb_transaction();
 
-	if (add_renormalize)
+	if (show_only && table_format) {
+		/* Prepare index and populate status */
+		wt_status_prepare(the_repository, &status);
+		git_config(git_default_config, &status);
+		repo_read_index(the_repository);
+		refresh_index(&the_index,
+			      REFRESH_QUIET|REFRESH_UNMERGED|progress_flag,
+			      &status.pathspec, NULL, NULL);
+		status.status_format = STATUS_FORMAT_TABLE;
+		status.show_branch = 0;
+	}
+
+	if (add_renormalize) {
 		exit_status |= renormalize_tracked_files(&pathspec, flags);
-	else
-		exit_status |= add_files_to_cache(the_repository, prefix,
+	} else {
+		exit_status |= add_files_to_cache_with_status(the_repository, prefix,
 						  &pathspec, include_sparse,
-						  flags);
+						  flags, &status);
+	}
 
-	if (add_new_files)
-		exit_status |= add_files(&dir, flags);
+	if (add_new_files) {
+		exit_status |= add_files(&dir, flags, &status);
+	}
+
+	if (show_only && table_format) {
+		wt_status_collect(&status);
+		wt_status_print(&status);
+		wt_status_collect_free_buffers(&status);
+	}
 
 	if (chmod_arg && pathspec.nr)
 		exit_status |= chmod_pathspec(&pathspec, chmod_arg[0], show_only);
diff --git a/builtin/commit.c b/builtin/commit.c
index 4338896dbf..33b15ef96e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -310,7 +310,7 @@ static void add_remove_files(struct string_list *list)
 			continue;
 
 		if (!lstat(p->string, &st)) {
-			if (add_to_index(&the_index, p->string, &st, 0))
+			if (add_file_to_index(&the_index, p->string, 0))
 				die(_("updating files failed"));
 		} else
 			remove_file_from_index(&the_index, p->string);
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 9a1a7edc5a..b1cee2c7ee 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -4,6 +4,7 @@
 #include "hash-ll.h"
 #include "hashmap.h"
 #include "statinfo.h"
+#include "wt-status.h"
 
 /*
  * Basic data structures for the directory cache
@@ -395,6 +396,7 @@ int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_IGNORE_ERRORS	4
 #define ADD_CACHE_IGNORE_REMOVAL 8
 #define ADD_CACHE_INTENT 16
+#define ADD_CACHE_FORMAT_TABLE 32
 /*
  * These two are used to add the contents of the file at path
  * to the index, marking the working tree up-to-date by storing
@@ -404,7 +406,8 @@ int remove_file_from_index(struct index_state *, const char *path);
  * the latter will do necessary lstat(2) internally before
  * calling the former.
  */
-int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
+int add_to_index(struct index_state *, const char *path, struct stat *, int flags, struct wt_status *status);
+int add_file_to_index_with_status(struct index_state *, const char *path, int flags, struct wt_status *status);
 int add_file_to_index(struct index_state *, const char *path, int flags);
 
 int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
@@ -475,6 +478,10 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, int include_sparse,
 		       int flags);
 
+int add_files_to_cache_with_status(struct repository *repo, const char *prefix,
+		       const struct pathspec *pathspec, int include_sparse,
+		       int flags, struct wt_status *status);
+
 void overlay_tree_on_index(struct index_state *istate,
 			   const char *tree_name, const char *prefix);
 
diff --git a/read-cache.c b/read-cache.c
index 080bd39713..e777cdb210 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -45,6 +45,8 @@
 #include "csum-file.h"
 #include "promisor-remote.h"
 #include "hook.h"
+#include "wt-status.h"
+#include "string-list.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 
@@ -664,7 +666,7 @@ void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)
 	oidcpy(&ce->oid, &oid);
 }
 
-int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
+int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags, struct wt_status *status)
 {
 	int namelen, was_same;
 	mode_t st_mode = st->st_mode;
@@ -672,6 +674,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE|CE_MATCH_RACY_IS_DIRTY;
 	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
 	int pretend = flags & ADD_CACHE_PRETEND;
+	int table = flags & ADD_CACHE_FORMAT_TABLE;
 	int intent_only = flags & ADD_CACHE_INTENT;
 	int add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|
 			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
@@ -760,17 +763,26 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 		discard_cache_entry(ce);
 		return error(_("unable to add '%s' to index"), path);
 	}
-	if (verbose && !was_same)
+	if (verbose && !was_same && !table)
 		printf("add '%s'\n", path);
+	if (table && pretend && !was_same) {
+		string_list_insert(&status->dry_run_added, path);
+	}
 	return 0;
 }
 
-int add_file_to_index(struct index_state *istate, const char *path, int flags)
+int add_file_to_index_with_status(struct index_state *istate, const char *path, int flags, struct wt_status *status)
 {
 	struct stat st;
 	if (lstat(path, &st))
 		die_errno(_("unable to stat '%s'"), path);
-	return add_to_index(istate, path, &st, flags);
+	return add_to_index(istate, path, &st, flags, status);
+}
+
+int add_file_to_index(struct index_state *istate, const char *path, int flags)
+{
+	struct wt_status status;
+	return add_file_to_index_with_status(istate, path, flags, &status);
 }
 
 struct cache_entry *make_empty_cache_entry(struct index_state *istate, size_t len)
@@ -3872,6 +3884,7 @@ struct update_callback_data {
 	int include_sparse;
 	int flags;
 	int add_errors;
+	struct wt_status *status;
 };
 
 static int fix_unmerged_status(struct diff_filepair *p,
@@ -3914,7 +3927,7 @@ static void update_callback(struct diff_queue_struct *q,
 			die(_("unexpected diff status %c"), p->status);
 		case DIFF_STATUS_MODIFIED:
 		case DIFF_STATUS_TYPE_CHANGED:
-			if (add_file_to_index(data->index, path, data->flags)) {
+			if (add_file_to_index_with_status(data->index, path, data->flags, data->status)) {
 				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
 					die(_("updating files failed"));
 				data->add_errors++;
@@ -3935,6 +3948,14 @@ static void update_callback(struct diff_queue_struct *q,
 int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, int include_sparse,
 		       int flags)
+{
+	struct wt_status status;
+	return add_files_to_cache_with_status(repo, prefix, pathspec, include_sparse, flags, &status);
+}
+
+int add_files_to_cache_with_status(struct repository *repo, const char *prefix,
+		       const struct pathspec *pathspec, int include_sparse,
+		       int flags, struct wt_status *status)
 {
 	struct update_callback_data data;
 	struct rev_info rev;
@@ -3943,6 +3964,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 	data.index = repo->index;
 	data.include_sparse = include_sparse;
 	data.flags = flags;
+	data.status = status;
 
 	repo_init_revisions(repo, &rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
diff --git a/table.c b/table.c
index 73751339da..a6fc660fec 100644
--- a/table.c
+++ b/table.c
@@ -65,18 +65,53 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
 		strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
 }
 
-static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+static void add_arrow_to_entry(struct strbuf *buf, int add_after_entry)
+{
+	struct strbuf empty = STRBUF_INIT;
+	struct strbuf trimmed = STRBUF_INIT;
+	struct strbuf holder = STRBUF_INIT;
+	int len = strlen(buf->buf);
+
+	strbuf_addstr(&trimmed, buf->buf);
+	strbuf_trim(&trimmed);
+
+	if (!strbuf_cmp(&trimmed, &empty) && !add_after_entry) {
+		strbuf_reset(buf);
+		strbuf_addchars(buf, '-', len + 1);
+	} else if (add_after_entry) {
+		strbuf_rtrim(buf);
+		strbuf_addchars(buf, ' ', 1);
+		strbuf_addchars(buf, '-', len - strlen(buf->buf) + 1);
+	} else if (!add_after_entry) {
+		strbuf_ltrim(buf);
+		strbuf_addchars(&holder, '-', len - strlen(buf->buf) - 2);
+		strbuf_addchars(&holder, '>', 1);
+		strbuf_addchars(&holder, ' ', 1);
+		strbuf_addstr(&holder, buf->buf);
+		strbuf_reset(buf);
+		strbuf_addstr(buf, holder.buf);
+	}
+}
+
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s, int hide_pipe)
 {
 	printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
-	printf(_("|"));
+	if (hide_pipe != 1 && hide_pipe != 3)
+		printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
-	printf(_("|"));
+	if (hide_pipe != 2 && hide_pipe != 3)
+		printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
 	printf(_("|\n"));
 }
 
-void build_and_draw_status_table(struct wt_status *s, int add_advice)
+static void print_table_body_line_(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+{
+	print_table_body_line(buf1, buf2, buf3, s, 0);
+}
+
+void build_and_draw_status_table(struct wt_status *s, int advice)
 {
 	struct winsize w;
 	int cols;
@@ -84,7 +119,7 @@ void build_and_draw_status_table(struct wt_status *s, int add_advice)
 	struct strbuf table_col_entry_1 = STRBUF_INIT;
 	struct strbuf table_col_entry_2 = STRBUF_INIT;
 	struct strbuf table_col_entry_3 = STRBUF_INIT;
-	struct string_list_item *item;
+	struct string_list_item *item, *item2;
 
 	/* Get terminal width */
 	ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
@@ -104,7 +139,7 @@ void build_and_draw_status_table(struct wt_status *s, int add_advice)
 	printf(_("%s\n"), table_border.buf);
 	printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
 
-	if (add_advice) {
+	if (advice) {
 		build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
 		build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
 		build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
@@ -122,14 +157,38 @@ void build_and_draw_status_table(struct wt_status *s, int add_advice)
 
 	/* Draw table body */
 	for_each_string_list_item(item, &s->untracked) {
-		build_table_entry(&table_col_entry_1, item->string, cols);
+		struct strbuf buf_1 = STRBUF_INIT;
+		struct strbuf buf_2 = STRBUF_INIT;
+		int is_arrow = 0;
+		strbuf_addstr(&buf_1, item->string);
+		build_table_entry(&table_col_entry_1, buf_1.buf, cols);
 		build_table_entry(&table_col_entry_2, "", cols);
 		build_table_entry(&table_col_entry_3, "", cols);
-		print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+		for_each_string_list_item(item2, &s->dry_run_added) {
+			strbuf_reset(&buf_2);
+			strbuf_addstr(&buf_2, item2->string);
+			if (!strbuf_cmp(&buf_1, &buf_2)) {
+				build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+				add_arrow_to_entry(&table_col_entry_1, 1);
+				add_arrow_to_entry(&table_col_entry_2, 0);
+				add_arrow_to_entry(&table_col_entry_3, 0);
+				is_arrow = 1;
+			}
+		}
+
+		if (!is_arrow)
+			print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+		else
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 3);
 	}
 
 	for_each_string_list_item(item, &s->change) {
 		struct wt_status_change_data *d = item->util;
+		struct strbuf buf_1 = STRBUF_INIT;
+		struct strbuf buf_2 = STRBUF_INIT;
+		int is_arrow = 0;
+		strbuf_addstr(&buf_1, item->string);
 		if (d->worktree_status && d->index_status) {
 			build_table_entry(&table_col_entry_1, "", cols);
 			build_table_entry(&table_col_entry_2, item->string, cols);
@@ -138,12 +197,27 @@ void build_and_draw_status_table(struct wt_status *s, int add_advice)
 			build_table_entry(&table_col_entry_1, "", cols);
 			build_table_entry(&table_col_entry_2, item->string, cols);
 			build_table_entry(&table_col_entry_3, "", cols);
+
+			for_each_string_list_item(item2, &s->dry_run_added) {
+				strbuf_reset(&buf_2);
+				strbuf_addstr(&buf_2, item2->string);
+				if (!strbuf_cmp(&buf_1, &buf_2)) {
+					build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+					add_arrow_to_entry(&table_col_entry_2, 1);
+					add_arrow_to_entry(&table_col_entry_3, 0);
+					is_arrow = 1;
+				}
+			}
 		} else if (d->index_status) {
 			build_table_entry(&table_col_entry_1, "", cols);
 			build_table_entry(&table_col_entry_2, "", cols);
 			build_table_entry(&table_col_entry_3, item->string, cols);
 		}
-		print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+		if (!is_arrow)
+			print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+		else
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 2);
 	}
 	
 	if (!s->untracked.nr && !s->change.nr) {
diff --git a/wt-status.c b/wt-status.c
index 62731859fe..975cfc01a5 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -153,6 +153,7 @@ void wt_status_prepare(struct repository *r, struct wt_status *s)
 	s->change.strdup_strings = 1;
 	s->untracked.strdup_strings = 1;
 	s->ignored.strdup_strings = 1;
+	s->dry_run_added.strdup_strings = 1;
 	s->show_branch = -1;  /* unspecified */
 	s->show_stash = 0;
 	s->ahead_behind_flags = AHEAD_BEHIND_UNSPECIFIED;
diff --git a/wt-status.h b/wt-status.h
index 70a3b7a2e4..5d29c058c1 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -142,6 +142,7 @@ struct wt_status {
 	struct string_list change;
 	struct string_list untracked;
 	struct string_list ignored;
+	struct string_list dry_run_added;
 	uint32_t untracked_in_ms;
 };
 
-- 
2.42.0.402.gbe8243af7b.dirty

[RFC PATCH 5/5] add: set unique color for -t, --table arrows

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 table.c     | 62 +++++++++++++++++++++++++++++++----------------------
 wt-status.c |  1 +
 wt-status.h |  1 +
 3 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/table.c b/table.c
index a6fc660fec..390b2e2dd9 100644
--- a/table.c
+++ b/table.c
@@ -5,6 +5,7 @@
 #include "wt-status.h"
 #include "config.h"
 #include "string-list.h"
+#include "color.h"
 #include "sys/ioctl.h"
 
 static const char *color(int slot, struct wt_status *s)
@@ -65,52 +66,51 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
 		strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
 }
 
-static void add_arrow_to_entry(struct strbuf *buf, int add_after_entry)
+static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry)
 {
 	struct strbuf empty = STRBUF_INIT;
 	struct strbuf trimmed = STRBUF_INIT;
-	struct strbuf holder = STRBUF_INIT;
 	int len = strlen(buf->buf);
 
+	strbuf_reset(arrow);
 	strbuf_addstr(&trimmed, buf->buf);
 	strbuf_trim(&trimmed);
 
 	if (!strbuf_cmp(&trimmed, &empty) && !add_after_entry) {
 		strbuf_reset(buf);
-		strbuf_addchars(buf, '-', len + 1);
+		strbuf_addchars(arrow, '-', len + 1);
 	} else if (add_after_entry) {
 		strbuf_rtrim(buf);
-		strbuf_addchars(buf, ' ', 1);
-		strbuf_addchars(buf, '-', len - strlen(buf->buf) + 1);
+		strbuf_addchars(arrow, ' ', 1);
+		strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
 	} else if (!add_after_entry) {
 		strbuf_ltrim(buf);
-		strbuf_addchars(&holder, '-', len - strlen(buf->buf) - 2);
-		strbuf_addchars(&holder, '>', 1);
-		strbuf_addchars(&holder, ' ', 1);
-		strbuf_addstr(&holder, buf->buf);
-		strbuf_reset(buf);
-		strbuf_addstr(buf, holder.buf);
+		strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
+		strbuf_addchars(arrow, '>', 1);
+		strbuf_addchars(arrow, ' ', 1);
 	}
 }
 
-static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s, int hide_pipe)
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct strbuf *arrow1, struct strbuf *arrow2, struct strbuf *arrow3, struct wt_status *s, int hide_pipe)
 {
 	printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
+	if (strlen(arrow1->buf) > 0)
+		color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow1->buf);
 	if (hide_pipe != 1 && hide_pipe != 3)
 		printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
+	if (strlen(arrow2->buf) > 0)
+		color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow2->buf);
 	if (hide_pipe != 2 && hide_pipe != 3)
 		printf(_("|"));
+	if (strlen(arrow3->buf) > 0) {
+		color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow3->buf);
+	}
 	color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
 	printf(_("|\n"));
 }
 
-static void print_table_body_line_(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
-{
-	print_table_body_line(buf1, buf2, buf3, s, 0);
-}
-
 void build_and_draw_status_table(struct wt_status *s, int advice)
 {
 	struct winsize w;
@@ -119,6 +119,9 @@ void build_and_draw_status_table(struct wt_status *s, int advice)
 	struct strbuf table_col_entry_1 = STRBUF_INIT;
 	struct strbuf table_col_entry_2 = STRBUF_INIT;
 	struct strbuf table_col_entry_3 = STRBUF_INIT;
+	struct strbuf arrow_1 = STRBUF_INIT;
+	struct strbuf arrow_2 = STRBUF_INIT;
+	struct strbuf arrow_3 = STRBUF_INIT;
 	struct string_list_item *item, *item2;
 
 	/* Get terminal width */
@@ -170,17 +173,21 @@ void build_and_draw_status_table(struct wt_status *s, int advice)
 			strbuf_addstr(&buf_2, item2->string);
 			if (!strbuf_cmp(&buf_1, &buf_2)) {
 				build_table_entry(&table_col_entry_3, buf_1.buf, cols);
-				add_arrow_to_entry(&table_col_entry_1, 1);
-				add_arrow_to_entry(&table_col_entry_2, 0);
-				add_arrow_to_entry(&table_col_entry_3, 0);
+				build_arrow(&table_col_entry_1, &arrow_1, 1);
+				build_arrow(&table_col_entry_2, &arrow_2, 0);
+				build_arrow(&table_col_entry_3, &arrow_3, 0);
 				is_arrow = 1;
 			}
 		}
 
 		if (!is_arrow)
-			print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
 		else
-			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 3);
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 3);
+
+		strbuf_reset(&arrow_1);
+		strbuf_reset(&arrow_2);
+		strbuf_reset(&arrow_3);
 	}
 
 	for_each_string_list_item(item, &s->change) {
@@ -203,8 +210,8 @@ void build_and_draw_status_table(struct wt_status *s, int advice)
 				strbuf_addstr(&buf_2, item2->string);
 				if (!strbuf_cmp(&buf_1, &buf_2)) {
 					build_table_entry(&table_col_entry_3, buf_1.buf, cols);
-					add_arrow_to_entry(&table_col_entry_2, 1);
-					add_arrow_to_entry(&table_col_entry_3, 0);
+					build_arrow(&table_col_entry_2, &arrow_2, 1);
+					build_arrow(&table_col_entry_3, &arrow_3, 0);
 					is_arrow = 1;
 				}
 			}
@@ -215,9 +222,12 @@ void build_and_draw_status_table(struct wt_status *s, int advice)
 		}
 
 		if (!is_arrow)
-			print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
 		else
-			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 2);
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 2);
+		strbuf_reset(&arrow_1);
+		strbuf_reset(&arrow_2);
+		strbuf_reset(&arrow_3);
 	}
 	
 	if (!s->untracked.nr && !s->change.nr) {
diff --git a/wt-status.c b/wt-status.c
index 975cfc01a5..fe38260baa 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -49,6 +49,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_GREEN,  /* WT_STATUS_LOCAL_BRANCH */
 	GIT_COLOR_RED,    /* WT_STATUS_REMOTE_BRANCH */
 	GIT_COLOR_NIL,    /* WT_STATUS_ONBRANCH */
+	GIT_COLOR_CYAN,   /* WT_STATUS_ARROW */
 };
 
 static const char *color(int slot, struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 5d29c058c1..0517f81e1b 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -19,6 +19,7 @@ enum color_wt_status {
 	WT_STATUS_LOCAL_BRANCH,
 	WT_STATUS_REMOTE_BRANCH,
 	WT_STATUS_ONBRANCH,
+	WT_STATUS_ARROW,
 	WT_STATUS_MAXSLOT
 };
 
-- 
2.42.0.402.gbe8243af7b.dirty

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-20 20:39, Jacob Stopak wrote:
> This is a proposal / proof-of-concept for a new table-based output
> format for the git status command, and for dry runs (-n) of the git add
> command. This could be extended to create visual dry runs for other
> other commands like rm, mv, restore, stash, commit, and clean.

Huh, please don't get me wrong, but based on the examples provided 
below, I really think that's only wasted screen estate, providing little 
or no help in understanding the performed operations.

I appreciate your effort, but IMHO it makes little sense from the 
usability standpoint.

> For some context, earlier this year I released a tool called Git-Sim
> (https://github.com/initialcommit-com/git-sim) which allows users to do
> visual dry runs of many Git commands, which are rendered as high 
> quality
> output image files. Simulating commands like status, add, rm, mv, 
> restore,
> stash, and commit creates a table with 3 columns to represent the way 
> file
> changes "move around" as a result of the command being simulated.
> 
> I've gotten positive feedback from users about this visual approach to
> simulating git commands, which is more intuitive than pure terminal 
> text
> for both newer users to understand how git works and for visual people.
> 
> As a result, I was thinking of ways to integrate these types of visual
> formats directly into Git. A table-based output format with colored
> highlighting for the commands mentioned above is low hanging fruit.
> 
> Teach 'git status' the new -t, --table flag, which displays the status
> output in a 3-column table format, preserving terminology and color
> coding from the default git status "long output" format (note that the
> column headers are shortened here for the small width of this email, 
> and
> also I just realized that the tables below might not look right on the
> mailing list due to the differing character width, but it looks correct
> in the terminal so please test there it's more fun anyway :D):
> 
> $ git status -t
> -------------------------------------------------------------------------
> |    Untracked files    | Changes n...or commit | Changes t...committed 
> |
> -------------------------------------------------------------------------
> |         poiu          |                       |                       
> |
> |     status-table/     |                       |                       
> |
> |                       |                       |         asdf          
> |
> |                       |        table.c        |                       
> |
> |                       |      wt-status.c      |                       
> |
> -------------------------------------------------------------------------
> 
> Teach 'git add' the new -t, --table flag to be used ONLY in combination
> with the '-n' flag for dry runs. Instead of simply printing out the
> added filenames, the full status table format is displayed, along with
> arrows that visually show how the added files are being moved around:
> 
> $ git add -nt poiu wt-status.c
> -------------------------------------------------------------------------
> |    Untracked files    | Changes n...or commit | Changes t...committed 
> |
> -------------------------------------------------------------------------
> |         poiu -----------------------------------------> poiu          
> |
> |     status-table/     |                       |                       
> |
> |                       |                       |         asdf          
> |
> |                       |        table.c        |                       
> |
> |                       |      wt-status.c ----------> wt-status.c      
> |
> -------------------------------------------------------------------------
> 
> Other notes:
> 
> * The width of the table and columns are dynamically set based on the
>   width of the terminal.
> 
> * Long paths are shortened to include the maximum number of characters
>   from both ends of the path that will fit, with a '...' in the middle.
> 
> * Color coding matches the default output of 'git status', with
>   untracked files and working dir mods in red, and staged changes in
>   green. If needed, arrows are drawn in cyan.
> 
> As stated above, the dry run version of the table format can be applied
> to various other commands like rm, mv, restore, stash, commit, and 
> clean
> which all move file changes around in a way that can be represented in
> the table format. New columns may need to be added or arrows reversed
> to show changes moving in various directions. Note that some of these
> commands don't appear to have a dry run (-n) option yet, so it could be
> added for consistency (if not already in use) and for use with the new
> table format.
> 
> Since this is an RFC patch series, I probably did some illegal and dumb
> things in my code changes just to get it into a demo-able state. I am a
> bit wary of having made changes to files like "read-cache.c" and
> "read-cache-ll.h" to pass in the wt_status info, and there are probably
> betters ways to do some other things too.
> 
> Feedback on both the new format itself and the implementation is very
> much appreciated!
> 
> Jacob Stopak (5):
>   status: introduce -t, --table flag
>   status: handle long paths with -t, --table flag
>   status: add advice arg for -t, --table flag
>   add: add -t, --table flag for visual dry runs
>   add: set unique color for -t, --table arrows
> 
>  Makefile         |   1 +
>  builtin/add.c    |  46 +++++++--
>  builtin/commit.c |   4 +-
>  read-cache-ll.h  |   9 +-
>  read-cache.c     |  32 ++++++-
>  table.c          | 245 +++++++++++++++++++++++++++++++++++++++++++++++
>  table.h          |   6 ++
>  wt-status.c      |  74 +++++++++-----
>  wt-status.h      |   3 +
>  9 files changed, 378 insertions(+), 42 deletions(-)
>  create mode 100644 table.c
>  create mode 100644 table.h

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

On Fri, Oct 20, 2023 at 08:48:12PM +0200, Dragan Simic wrote:
> On 2023-10-20 20:39, Jacob Stopak wrote:
> > This is a proposal / proof-of-concept for a new table-based output
> > format for the git status command, and for dry runs (-n) of the git add
> > command. This could be extended to create visual dry runs for other
> > other commands like rm, mv, restore, stash, commit, and clean.
> 
> Huh, please don't get me wrong, but based on the examples provided below, I
> really think that's only wasted screen estate, providing little or no help
> in understanding the performed operations.
> 
> I appreciate your effort, but IMHO it makes little sense from the usability
> standpoint.
> 

Thanks for the quick (and honest ;) reply - I appreciate it and no offense
taken! But let me try to expand on my reasoning a bit.

I agree with you that Git users who are already comfortable with Git,
the command-line, and their workflows would be unlikely to use this in
their day to day work.

The main benefits of this format are for beginners and folks who
are still learning Git to use it as needed:

  * To beginners, the concepts of working directory and "staging area"
    can be very abstract. By representing these concepts as table columns
    on the screen, (a format that 99% of humans are used to interpreting),
    they become more tangible and intuitive to new users.

  * In Git, changes fly around all over the place, in all sorts of
    directions. Even small hints at this movement can be very helpful to
    understand what the heck is going on. The table format (esp with
    arrows used in the 'git add' version) highlights the "flow" of
    changes through the workflow in a way that the current default format
    doesn't. The current dry runs just show the filenames being added
    without context of _where_ they come from and where they are going.
    Not to mention many commands don't even have dry runs. This might
    sound like a small thing, but to a newbie having that extra level of
    confirmation and understanding can make a big difference.

  * Git doesn't exactly have a reputation as a user-friendly tool, and
    much of that stems from the difficulty of learning Git. So we should
    try to make it more approachable to normal humans. This format
    (esp if applied to a wide variety of commands as dry runs) would
    provide a rudimentary visual output that is more intuitive to users.

  * This flag doesn't change any default behavior, it can easily be
    tossed on for newbie use (either when teaching a newbie or when the
    newbie is practicing on their own). Given this usage, the screen
    realestate is not really a concern. I.e. this would be used
    specifically when needed for the extra info/clarity it provides,
    not to be efficient with the terminal space.

That's my perspective anyway, but of course the point of this is to
propose it to the community and hear the response, so even if it's
not included it's still a good experience :D.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-20 23:48, Jacob Stopak wrote:
> On Fri, Oct 20, 2023 at 08:48:12PM +0200, Dragan Simic wrote:
>> On 2023-10-20 20:39, Jacob Stopak wrote:
>> > This is a proposal / proof-of-concept for a new table-based output
>> > format for the git status command, and for dry runs (-n) of the git add
>> > command. This could be extended to create visual dry runs for other
>> > other commands like rm, mv, restore, stash, commit, and clean.
>> 
>> Huh, please don't get me wrong, but based on the examples provided 
>> below, I
>> really think that's only wasted screen estate, providing little or no 
>> help
>> in understanding the performed operations.
>> 
>> I appreciate your effort, but IMHO it makes little sense from the 
>> usability
>> standpoint.
>> 
> Thanks for the quick (and honest ;) reply - I appreciate it and no 
> offense
> taken! But let me try to expand on my reasoning a bit.

Thank you!

> I agree with you that Git users who are already comfortable with Git,
> the command-line, and their workflows would be unlikely to use this in
> their day to day work.
> 
> The main benefits of this format are for beginners and folks who
> are still learning Git to use it as needed:

Oh, I always do my best to put myself in the shoes of the targeted 
audience.  Maybe I sometimes fail at that, I don't know, but that's why 
we're here to discuss it further.

>   * To beginners, the concepts of working directory and "staging area"
>     can be very abstract. By representing these concepts as table 
> columns
>     on the screen, (a format that 99% of humans are used to 
> interpreting),
>     they become more tangible and intuitive to new users.

Frankly, based on my rather broad experience, there are two primary 
categories of the beginners in the world of version control software 
(VCS), be it git or any other product:

1) People who are forced to use some VCS at work, and they actually 
don't give a damn about it.
2) True enthusiasts who love what they do, and who love expanding their 
knowledge.

For the first category, nothing helps.  For the second category, a 
nicely written tutorial is all they needed to start with, aided later 
with the man pages, Stack Exchange, and perhaps some textbook.

>   * In Git, changes fly around all over the place, in all sorts of
>     directions. Even small hints at this movement can be very helpful 
> to
>     understand what the heck is going on. The table format (esp with
>     arrows used in the 'git add' version) highlights the "flow" of
>     changes through the workflow in a way that the current default 
> format
>     doesn't. The current dry runs just show the filenames being added
>     without context of _where_ they come from and where they are going.
>     Not to mention many commands don't even have dry runs. This might
>     sound like a small thing, but to a newbie having that extra level 
> of
>     confirmation and understanding can make a big difference.

Please don't get me wrong, I understand your reasoning, but again, it 
all comes down to the two categories described above.  IMHO, the second 
category will likely start turning off the default hints sooner than 
turning the table formatting on.  The first category will choose some 
GUI anyway.

>   * Git doesn't exactly have a reputation as a user-friendly tool, and
>     much of that stems from the difficulty of learning Git. So we 
> should
>     try to make it more approachable to normal humans. This format
>     (esp if applied to a wide variety of commands as dry runs) would
>     provide a rudimentary visual output that is more intuitive to 
> users.

No pain, no gain.  That's the ancient mantra, but IMHO it still applies 
very well to many things, and of course not to the first category 
mentioned above.  Nothing applies to that category.

>   * This flag doesn't change any default behavior, it can easily be
>     tossed on for newbie use (either when teaching a newbie or when the
>     newbie is practicing on their own). Given this usage, the screen
>     realestate is not really a concern. I.e. this would be used
>     specifically when needed for the extra info/clarity it provides,
>     not to be efficient with the terminal space.

As I already assumed above, the targeted audience will likely start 
turning the default hints off, rather than turning the table formatting 
on.  Maybe I'm wrong there, who knows.

> That's my perspective anyway, but of course the point of this is to
> propose it to the community and hear the response, so even if it's
> not included it's still a good experience :D.

Let's hear more thoughts from other people, of course.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Junio C Hamano]

Dragan Simic <dsimic@manjaro.org> writes:

> Please don't get me wrong, I understand your reasoning, but again, it
> all comes down to the two categories described above.  IMHO, the
> second category will likely start turning off the default hints sooner
> than turning the table formatting on.  The first category will choose
> some GUI anyway.

You are not alone in feeling the impedance mismatch between the
intended audience the patch(es) try to help (pointy-clicky GUI
users) and the codebase the patch(es) modify (perhaps spartan
command line interface).  I did wonder why this is not made as a
part of sugarcoating the command line interface with some GUI that
shows what could be added, what has been added, and the stuff in its
"git status" equivalent.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

On Fri, Oct 20, 2023 at 04:28:19PM -0700, Junio C Hamano wrote:
> 
> You are not alone in feeling the impedance mismatch between the
> intended audience the patch(es) try to help (pointy-clicky GUI
> users) 

I'm sure there's overlap with "pointy-clicky GUI users" but my point
isn't to directly cater to them. I find it intersting to think about
how visual (and ok fine even gui) tools can be used as bridge tools
that can be discarded one the important concepts are solidified, and
maybe resurrected in a moment of stupidity or strife.

It's like yes use the crutch if you need it, but then do it the real
way once you get it.

And altho this is a visual helper feature, it keeps the user within the
terminal, close to the Git cli and may help some subset stay there.

> and the codebase the patch(es) modify (perhaps spartan
> command line interface).

Git does have some comforting features doesn't it? For example the
hints, as well as the nice pretty colored --graph option for log. I'm
sure I'm missing some others? Isn't there a file called pretty.c?! :D

> I did wonder why this is not made as a
> part of sugarcoating the command line interface with some GUI that
> shows what could be added, what has been added, and the stuff in its
> "git status" equivalent.

I'm working on a couple of tools (ok fine basically guis) that include
this feature.

The reason to bring it up here is that a common feedback I got on my
existing tool was "add something like this into git", so I was curious
about what was possible to do from the Git cli.

This would obviously be the best way for the feature to reach the widest
possible audience and get the most use. Any standalone tool I create
would get a teensy fraction of the use that a feature in Git itself would
get, so I figured I'd give it a whirl.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-22 08:04, Jacob Stopak wrote:
> On Fri, Oct 20, 2023 at 04:28:19PM -0700, Junio C Hamano wrote:
>> 
>> You are not alone in feeling the impedance mismatch between the
>> intended audience the patch(es) try to help (pointy-clicky GUI
>> users)
> 
> I'm sure there's overlap with "pointy-clicky GUI users" but my point
> isn't to directly cater to them. I find it intersting to think about
> how visual (and ok fine even gui) tools can be used as bridge tools
> that can be discarded one the important concepts are solidified, and
> maybe resurrected in a moment of stupidity or strife.
> 
> It's like yes use the crutch if you need it, but then do it the real
> way once you get it.

Quite frankly, that would be like starting to learn how to drive a car 
by playing GTA 5, or whichever version of GTA it currently popular.  I 
don't think that would work out well for the vast majority of student 
drivers.

> And altho this is a visual helper feature, it keeps the user within the
> terminal, close to the Git cli and may help some subset stay there.

Even if that would work out for some people, it would require the 
formatting into tables to be the default for git, which frankly I'd 
never support.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

> Frankly, based on my rather broad experience, there are two primary
> categories of the beginners in the world of version control software (VCS),
> be it git or any other product:
> 
> 1) People who are forced to use some VCS at work, and they actually don't
> give a damn about it.
> 2) True enthusiasts who love what they do, and who love expanding their
> knowledge.
>
> For the first category, nothing helps.  

Interesting categorization I didn't think of splitting users that way. I
guess for group 1 that's true, if they are shown a GUI and can run 3
commands that can do what they need, that's all they will ever use.

> For the second category, a nicely
> written tutorial is all they needed to start with, aided later with the man
> pages, Stack Exchange, and perhaps some textbook.

This is the exact way I learned Git and became comfortable and eventually
confident using it. Reflecting on that, I really only started to become
truly confident after understanding the core underlying concepts (maybe
this is obvious / true for anything). And it's always easy once you get it.

However, there is one main benefit of a feature like this, that none of
the other options (man pages, stack exchange, a textbook) can provide:

Since the tool (Git) has access to and knows the exact state of your local
environment, it can provide instant feedback that takes into account that
context. That is immeasurably more helpful than trying to figure out how
to best ask Google your question, and then piecing together your problem
with a similar one some lost soul ran into 10 years ago.

> Please don't get me wrong, I understand your reasoning, but again, it all
> comes down to the two categories described above.  IMHO, the second category
> will likely start turning off the default hints sooner than turning the
> table formatting on.  The first category will choose some GUI anyway.

The default hints are an intersting consideration. I've found them handy
for commands that I use infrequently, and also when I find myself in a
scenario that is not a part of my usual workflow.

And the hint feature does show that Git has some "helper" features to
hold the user's hand at least a little bit.

> No pain, no gain.  That's the ancient mantra, but IMHO it still applies very
> well to many things, and of course not to the first category mentioned
> above.  Nothing applies to that category.

Somehow I do feel some sense of satisfaction at the countless times I've
I've been stuck on some menial issue only to find out it had a stupid
solution I overlooked. It's also just kind of funny in hindsight.

Regardless of a table formatting feature in Git, there is still plenty of
sweet sweet pain to be had with software dev in general.

But in the moment I always do appreciate being able to get past a
roadblock as quickly as possible. I would want a tool I design to be
known for avoiding pain rather than causing it.

> As I already assumed above, the targeted audience will likely start turning
> the default hints off, rather than turning the table formatting on.  Maybe
> I'm wrong there, who knows.

Even if the hints are off (presumably because the user felt confident
enough or annoyed enough to turn them off), sometimes people just run
into a situation they need an extra bit of clarification on.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-22 07:52, Jacob Stopak wrote:
>> Frankly, based on my rather broad experience, there are two primary
>> categories of the beginners in the world of version control software 
>> (VCS),
>> be it git or any other product:
>> 
>> 1) People who are forced to use some VCS at work, and they actually 
>> don't
>> give a damn about it.
>> 2) True enthusiasts who love what they do, and who love expanding 
>> their
>> knowledge.
>> 
>> For the first category, nothing helps.
> 
> Interesting categorization I didn't think of splitting users that way. 
> I
> guess for group 1 that's true, if they are shown a GUI and can run 3
> commands that can do what they need, that's all they will ever use.

Coincidentally, yesterday I was demonstrated for the first time in my 
life the way VS Code works with git, by a member of the first category 
of users.  It's dumbed down to the extreme, hiding pretty much all the 
details and git specifics, which is exactly what the first category 
wants.  Now I understand why the VS Code is so much popular.

The second category, myself included, tends to be genuinely disgusted by 
such an approach that makes everything nearly sterile.  But I do 
understand why many users simply love it that way.  Maybe I digress.

>> For the second category, a nicely
>> written tutorial is all they needed to start with, aided later with 
>> the man
>> pages, Stack Exchange, and perhaps some textbook.
> 
> This is the exact way I learned Git and became comfortable and 
> eventually
> confident using it. Reflecting on that, I really only started to become
> truly confident after understanding the core underlying concepts (maybe
> this is obvious / true for anything). And it's always easy once you get 
> it.

I agree, understanding the internals of some project or product, with 
many or all of its outer layers peeled back, is often the only way to 
really get to know it.

> However, there is one main benefit of a feature like this, that none of
> the other options (man pages, stack exchange, a textbook) can provide:
> 
> Since the tool (Git) has access to and knows the exact state of your 
> local
> environment, it can provide instant feedback that takes into account 
> that
> context. That is immeasurably more helpful than trying to figure out 
> how
> to best ask Google your question, and then piecing together your 
> problem
> with a similar one some lost soul ran into 10 years ago.

True, but I still think that having git put its thoughts into tables is 
actually not helpful.  To be precise, it actually might be helpful, but 
only to the first category of users, who will never reach it.  I mean, 
never say never, but in this case I'm pretty sure it's safe to say it.  
Unfortunately.

>> Please don't get me wrong, I understand your reasoning, but again, it 
>> all
>> comes down to the two categories described above.  IMHO, the second 
>> category
>> will likely start turning off the default hints sooner than turning 
>> the
>> table formatting on.  The first category will choose some GUI anyway.
> 
> The default hints are an intersting consideration. I've found them 
> handy
> for commands that I use infrequently, and also when I find myself in a
> scenario that is not a part of my usual workflow.

The built-in hints are useful without doubt, and in fact I still have at 
least a dozen of them left enabled.

> And the hint feature does show that Git has some "helper" features to
> hold the user's hand at least a little bit.

IMHO, git strikes a very good balance between holding the user's hand 
and leaving them on their own.  For the second category of users, of 
course.

>> No pain, no gain.  That's the ancient mantra, but IMHO it still 
>> applies very
>> well to many things, and of course not to the first category mentioned
>> above.  Nothing applies to that category.
> 
> Somehow I do feel some sense of satisfaction at the countless times 
> I've
> I've been stuck on some menial issue only to find out it had a stupid
> solution I overlooked. It's also just kind of funny in hindsight.
> 
> Regardless of a table formatting feature in Git, there is still plenty 
> of
> sweet sweet pain to be had with software dev in general.
> 
> But in the moment I always do appreciate being able to get past a
> roadblock as quickly as possible. I would want a tool I design to be
> known for avoiding pain rather than causing it.

I agree, software in general shouldn't cause people pain, it should make 
people's lives better.  However, many people expect software these days 
to be some kind of pain killer, which it simply can't be unless dumbed 
down to the extreme.  If you ask any doctor what results from taking 
pain killers for an extended period of time, they'll answer you that 
stronger pain killers will usually become needed.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Oswald Buddenhagen]

On Sun, Oct 22, 2023 at 08:38:19AM +0200, Dragan Simic wrote:
>True, but I still think that having git put its thoughts into tables is 
>actually not helpful.
>
i'm not convinced that the proposed feature specifically would have 
helped me, either (i found the index a rather obvious concept once i 
knew that it's there), but i'm making a general argument here. so:

>To be precise, it actually might be helpful, but only to the first 
>category of users, who will never reach it.  I mean, never say never, 
>but in this case I'm pretty sure it's safe to say it.  
>
well, and i think that you're wrong about that.
your categorization is simply wrong, because it assumes an incorrect 
static model.

while for the last decade i've been as much of a git expert as one can 
reasonably be without being literally obsessed with it or having written 
much of it, i absolutely *did* start out in your first category (as in, 
it was forced upon me, while i couldn't have cared less about the 
specifics - p4 was working well enough (or so i thought)). and i hated 
this stupid git (it was 2009, and it was much more of a pita for noobs 
than it is now). i certainly could have used more sensible 
visualizations at every step - on the command line, because that's where 
i mostly "live".

the second major error in the thinking is that "expert" and "gui user" 
are mutually exclusive categories. while i do most things on the command 
line, i would never voluntarily use "add -p" - why should i inflict that 
pain upon me, when i can simply use git-gui to do the job in a much more 
visual and freely navigable way? the same goes for "log --graph" vs.  
gitk, and git's "blame" function vs. qt creator's (or git-gui's, but i 
don't use it for that).

regards

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-22 12:30, Oswald Buddenhagen wrote:
> On Sun, Oct 22, 2023 at 08:38:19AM +0200, Dragan Simic wrote:
>> True, but I still think that having git put its thoughts into tables
>> is actually not helpful.
> 
> i'm not convinced that the proposed feature specifically would have
> helped me, either (i found the index a rather obvious concept once i
> knew that it's there), but i'm making a general argument here. so:
> 
>> To be precise, it actually might be helpful, but only to the first
>> category of users, who will never reach it.  I mean, never say never,
>> but in this case I'm pretty sure it's safe to say it.
> 
> well, and i think that you're wrong about that.
> your categorization is simply wrong, because it assumes an incorrect
> static model.
> 
> while for the last decade i've been as much of a git expert as one can
> reasonably be without being literally obsessed with it or having
> written much of it, i absolutely *did* start out in your first
> category (as in, it was forced upon me, while i couldn't have cared
> less about the specifics - p4 was working well enough (or so i
> thought)). and i hated this stupid git (it was 2009, and it was much
> more of a pita for noobs than it is now). i certainly could have used
> more sensible visualizations at every step - on the command line,
> because that's where i mostly "live".

Oh, that's awesome and I'm really happy to be wrong with my broad 
classification of VCS users.  However, I still need to be convinced 
further, and I'd assign your example as an exception to the rules, 
especially because you migrated to git from another VCS, which you 
liked, and because you use the command line a lot.

Full disclosure, I used Subversion for many years and I loved it.  I 
knew it very well and it did all I needed for me and the team I worked 
with.  Then git came and I really didn't like it, because it was touted 
to be "the best thing ever".  After using git for a while, I can firmly 
say that git is awesome, but that it also is a total overkill for many 
projects that need a VCS, for which choosing Subversion would be a much 
batter choice.  Why, you'll ask?  Because Subversion is many times 
simpler, and because many projects actually don't need a distributed 
VCS.

> the second major error in the thinking is that "expert" and "gui user"
> are mutually exclusive categories. while i do most things on the
> command line, i would never voluntarily use "add -p" - why should i
> inflict that pain upon me, when i can simply use git-gui to do the job
> in a much more visual and freely navigable way? the same goes for "log
> --graph" vs.  gitk, and git's "blame" function vs. qt creator's (or
> git-gui's, but i don't use it for that).

I also ask myself why would I use git-gui or any other GUI utility?  To 
me, clicking on something that represents a file is often simply wrong.  
Though, I understand that many people prefer GUI utilities and I respect 
that, everyone is free to do anything, but I also expect others to 
respect my own preferences.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Oswald Buddenhagen]

On Sun, Oct 22, 2023 at 02:55:05PM +0200, Dragan Simic wrote:
>Oh, that's awesome and I'm really happy to be wrong with my broad 
>classification of VCS users.  However, I still need to be convinced 
>further, and I'd assign your example as an exception to the rules, 
>
i don't see myself as exceptional at all in that regard.
in fact, your second user group seems like unicorns, and the first like 
a disparaging attitude from an elitist. in reality, users lie on a 
spectrum of willingness to engage with the details of the tools they 
use, and that willingness is circumstantial. a tool that is forthcoming 
with information has a higher chance of being actively engaged.

> especially because you migrated to git from another VCS,
>
that isn't all that uncommon, esp. in the older cohorts. and unless git 
achieves a total monopoly, it will remain a regular occurence.

but i don't see how that advances your argument anyway.

> which you liked,
>
i didn't say that.

> and because you use the command line a lot.
>
in my experience, this isn't uncommon for users of "discrete" vcs'es at 
all, even if they aren't too interested in the details. they just copy 
"magic incantations" from stackoverflow, etc. - disgusting, right? ;-)

>After using git for a while, I can firmly say that git is awesome, but 
>that it also is a total overkill for many projects that need a VCS, for 
>which choosing Subversion would be a much batter choice.  Why, you'll 
>ask?  Because Subversion is many times simpler, and because many 
>projects actually don't need a distributed VCS.
>
that line of reasoning seems mostly bogus to me. every project can 
benefit from using a dvcs - reviewing and polishing the history prior to 
publication leads to higher quality (primarily of the history, but such 
polishing often also transpires to the code itself), so encouraging it 
is a useful default (of course interested users can just use git-svn, 
but that's a bigger step than just having a closer look at what was 
shoved in their faces).

git is indeed pretty much by definition many times harder than svn, 
simply because it splits the process of submission into three stages.  
however, this is not a _useful_ definition, as everyone can remember to 
use `git commit -a && git push` instead of `svn commit`. the real 
complexity comes from all the interesting things one can do because of 
the split stages, but that's optional (well, until you need to pull and 
you get a merge conflict - unlike with svn, git leaves the repo in a 
"weird" state, and the poor communication of that was in fact the major 
source of frustration for me when i started).

>I also ask myself why would I use git-gui or any other GUI utility?  To 
>me, clicking on something that represents a file is often simply wrong.  
>
that makes you an outlier. most people find point-and-click interaction 
rather intuitive and significantly more efficient than encoding their 
intent into character sequences.

also, i said "add -p". selecting hunks (and even single lines) plays in 
a wholly different league than whole files.

>Though, I understand that many people prefer GUI utilities and I 
>respect that, everyone is free to do anything, but I also expect others 
>to respect my own preferences.
>
where did anyone even suggest disrespecting your preferences?

you should however consider whether your preferences are a good default 
for the wider audience, even within the context of the command line.

i for one think that it would be a perfectly valid experiment to go 
all-in and beyond with jacob's proposal - _and make it the default_ 
(when the output is a tty). more advanced users who feel annoyed would 
be expected to opt out of it via configuration, as they are for the 
advice messages. because it's really the same idea, only thought bigger.

regards

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-23 12:52, Oswald Buddenhagen wrote:
> On Sun, Oct 22, 2023 at 02:55:05PM +0200, Dragan Simic wrote:
>> Oh, that's awesome and I'm really happy to be wrong with my broad
>> classification of VCS users.  However, I still need to be convinced
>> further, and I'd assign your example as an exception to the rules,
> 
> i don't see myself as exceptional at all in that regard.
> in fact, your second user group seems like unicorns, and the first
> like a disparaging attitude from an elitist. in reality, users lie on
> a spectrum of willingness to engage with the details of the tools they
> use, and that willingness is circumstantial. a tool that is
> forthcoming with information has a higher chance of being actively
> engaged.

Actually, I see myself as some kind of a slave worker who just keeps 
typing on his keyboard and helps the elite (i.e. the "normal people") to 
enjoy their lives.  In other words, my viewpoint is totally opposite of 
how you perceived it.

>> and because you use the command line a lot.
>> 
> in my experience, this isn't uncommon for users of "discrete" vcs'es
> at all, even if they aren't too interested in the details. they just
> copy "magic incantations" from stackoverflow, etc. - disgusting,
> right? ;-)

Good point.  Various commands are often simply copied and pasted with 
little understanding.  I guess that makes people content, as one of 
their life choices.  I can respect that.

>> I also ask myself why would I use git-gui or any other GUI utility?
>> To me, clicking on something that represents a file is often simply
>> wrong.
> 
> that makes you an outlier. most people find point-and-click
> interaction rather intuitive and significantly more efficient than
> encoding their intent into character sequences.

Well, I guess I'm different.  As I already wrote above, I see myself as 
some kind of a "slave worker" who helps in enabling others (i.e. the 
"elite") to do whatever they want.

> you should however consider whether your preferences are a good
> default for the wider audience, even within the context of the command
> line.

Actually, some of my own preferences for my environment, when it comes 
to the git configuration, are not the defaults.

> i for one think that it would be a perfectly valid experiment to go
> all-in and beyond with jacob's proposal - _and make it the default_
> (when the output is a tty). more advanced users who feel annoyed would
> be expected to opt out of it via configuration, as they are for the
> advice messages. because it's really the same idea, only thought
> bigger.

I'd never support that, FWIW.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

On Mon, Oct 23, 2023 at 04:34:15PM +0200, Dragan Simic wrote:
> On 2023-10-23 12:52, Oswald Buddenhagen wrote:
> > On Sun, Oct 22, 2023 at 02:55:05PM +0200, Dragan Simic wrote:
> > > Oh, that's awesome and I'm really happy to be wrong with my broad
> > > classification of VCS users.  However, I still need to be convinced
> > > further, and I'd assign your example as an exception to the rules,
> > 
> > i don't see myself as exceptional at all in that regard.
> > in fact, your second user group seems like unicorns, and the first
> > like a disparaging attitude from an elitist. in reality, users lie on
> > a spectrum of willingness to engage with the details of the tools they
> > use, and that willingness is circumstantial. a tool that is
> > forthcoming with information has a higher chance of being actively
> > engaged.

Just a note here, in my initial reply I was thinking of writing something
similar about how in reality users of a tool as ubiquitous as Git would
form a continuous spectrum in terms of their usage habits and trying to
neatly plop them into 2 categories by speculating on their motives is
an oversimplification to the point where it might not be so helpful
evaluating whether an option like this would make sense to implement.

To me the bigger question is much simpler:

"Would this feature improve the Git experience for a signficant number
of users?"

I have some evidence to support the claim that it would.

My Git-Sim tool does essentially what this proposal suggests and it has
about 31,000 installs since I released it early this year. Granted this
is a drop in the bucket in the grand scheme of things, but it still shows
that there is demand for such a thing.

Git-Sim is a visual dry-run tool for Git that creates images simulating
what the corresponding Git command will do, without actually making any
change to the underlying repo state. Another important aspect is that
command syntax mimics Git's exactly - so to simulate any Git command, like:

$ git add asdf.txt qwer.txt

You would just replace the executable name and run:

$ git-sim add asdf.txt qwer.txt

and it will show you in an image exactly what will happen.

This is important because even simulating the command requires the user
to know and use the Git CLI syntax for the command. It keeps them on the
command line to do all of their actual work, unlike other true "pointy
clicky GUI's" I've seen which expect the user to do all their work in the
GUI. In fact this tool and feature expect no pointing or clicking at all.

The purpose of this is that users actually do all their work in the CLI
and learn to use Git as intuitively as possible, the way the "spartan"
CLI folks use it, the way it is meant to be used.

The reason to include a format like this in Git instead of just in my
tool is simply to reach a wider audience and benefit more people. Of
course it also appears much more trustworthy when a feature is part of
the native tool itself instead of some external thing.

> > i for one think that it would be a perfectly valid experiment to go
> > all-in and beyond with jacob's proposal - _and make it the default_
> > (when the output is a tty). more advanced users who feel annoyed would
> > be expected to opt out of it via configuration, as they are for the
> > advice messages. because it's really the same idea, only thought
> > bigger.
> 
> I'd never support that, FWIW.

FWIW, I'd _never suggest_ that. I very much value Git's current usage
and wouldn't dream to make this the default. This proposal is for an
optional flag to help users who would benefit from it, nothing more,
nothing less. Speculating on user motives to classify them into 2 broad
categories in order to prove the feature isn't helpful misses the point
that there is a (relatively large IMO) subset of users who would benefit
from it.

As an optional flag, experienced users wouldn't bat an eyelash, and the
type of users who installed my tool could use the flag on and off until
they feel confident enough to drop it. But it is always there in case
they need a refresher.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-23 19:30, Jacob Stopak wrote:
> On Mon, Oct 23, 2023 at 04:34:15PM +0200, Dragan Simic wrote:
>> On 2023-10-23 12:52, Oswald Buddenhagen wrote:
>>> i for one think that it would be a perfectly valid experiment to go
>>> all-in and beyond with jacob's proposal - _and make it the default_
>>> (when the output is a tty). more advanced users who feel annoyed 
>>> would
>>> be expected to opt out of it via configuration, as they are for the
>>> advice messages. because it's really the same idea, only thought
>>> bigger.
>> 
>> I'd never support that, FWIW.
> 
> FWIW, I'd _never suggest_ that. I very much value Git's current usage
> and wouldn't dream to make this the default. This proposal is for an
> optional flag to help users who would benefit from it, nothing more,
> nothing less. Speculating on user motives to classify them into 2 broad
> categories in order to prove the feature isn't helpful misses the point
> that there is a (relatively large IMO) subset of users who would 
> benefit
> from it.
> 
> As an optional flag, experienced users wouldn't bat an eyelash, and the
> type of users who installed my tool could use the flag on and off until
> they feel confident enough to drop it. But it is always there in case
> they need a refresher.

Perhaps my broad classification of git users wasn't that great, and I'm 
actually happy for being wrong there.

Just to clarify, in general I'd support the inclusion of the table 
output formatting, but I'd need to test it in detail before saying a 
definite yes.  It would also need to be more feature-complete, and the 
original author of the patches should commit to maintaining it as the 
new git feature.  Having different options available can't hurt, IMHO.

However, having that as the default is something I'll never support.  
All this is just my opinion.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Oswald Buddenhagen]

On Mon, Oct 23, 2023 at 10:30:54AM -0700, Jacob Stopak wrote:
>On Mon, Oct 23, 2023 at 04:34:15PM +0200, Dragan Simic wrote:
>> On 2023-10-23 12:52, Oswald Buddenhagen wrote:
>> > i for one think that it would be a perfectly valid experiment to go
>> > all-in and beyond with jacob's proposal - _and make it the default_
>> 
>> I'd never support that, FWIW.
>
>FWIW, I'd _never suggest_ that.
>
why, though?
doing that would extend the feature's reach about two orders of 
magnitude among newbies, which is where it matters most.

>I very much value Git's current usage and wouldn't dream to make this 
>the default.
>
making the default output format somewhat more verbose wouldn't really 
"change the usage", though. and being able to permanently get rid of it 
with a single command should alleviate any _reasonable_ concerns about 
habit disruption.

regards

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

On Mon, Oct 23, 2023 at 08:16:42PM +0200, Oswald Buddenhagen wrote:
> On Mon, Oct 23, 2023 at 10:30:54AM -0700, Jacob Stopak wrote:
> > On Mon, Oct 23, 2023 at 04:34:15PM +0200, Dragan Simic wrote:
> > > On 2023-10-23 12:52, Oswald Buddenhagen wrote:
> > > > i for one think that it would be a perfectly valid experiment to go
> > > > all-in and beyond with jacob's proposal - _and make it the default_
> > > 
> > > I'd never support that, FWIW.
> > 
> > FWIW, I'd _never suggest_ that.
> > 
> why, though?
> doing that would extend the feature's reach about two orders of magnitude
> among newbies, which is where it matters most.

To be honest it never even popped into my head to contemplate that and
how the user experience might be impacted by making it the default. I
assume the big distinction is "would it help more users than it would
annoy"?

I always saw this feature as a helper to be invoked when the user is in need
as opposed to a default, similar to the -n dry run option on some commands.

For brand new users, I can see what you mean since they would likely not
know the --table format exists unless being instructed by someone else.
It would be kindof a shame for this capability to exist but not be taken
advantage of by the folks who need it most - the newbies running "git
status" literally for the very first time.

But the main drawback is that although the --table for status does provide
some visual clarity and tangibility, the status command doesn't actually
_do_ anything, so the arrows showing how changes move around don't apply.

Those arrows showing how things move only really apply to "simulating"
(dry runs) for specific commands like add, restore, rm, commit, stash,
etc, so making the --table proposal a default status output would still
miss those scenarios.

However, now that I'm thinking about it maybe it could somehow be included
in the Hints feature? I honestly don't know exactly when the hints are
currently invoked or how much detail they go into, but what just popped
into my head is kindof a "universal dry run" option, which would show the
user the --table format hint when they invoke an applicable command, and
prompt them if they actually want to run it.

> 
> > I very much value Git's current usage and wouldn't dream to make this
> > the default.
> > 
> making the default output format somewhat more verbose wouldn't really
> "change the usage", though. and being able to permanently get rid of it with
> a single command should alleviate any _reasonable_ concerns about habit
> disruption.
> 
> regards

It's a good point too that people surprised or annoyed or disgusted by the
change of a longstanding status output format could just disable the
configuration with a single config command...

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Oswald Buddenhagen]

On Mon, Oct 23, 2023 at 12:29:12PM -0700, Jacob Stopak wrote:
>Those arrows showing how things move only really apply to "simulating"
>(dry runs) for specific commands like add, restore, rm, commit, stash,
>etc, so making the --table proposal a default status output would still
>miss those scenarios.
>
you're too focused on the status quo of your own tool. :-)
there is really nothing that would speak against the real commands 
reporting what they just *actually did*. this would seem rather helpful 
for noobs and other insecure users.
if one really wanted, "you can also use this with --dry-run" could be 
part of the hint that would say how to turn off the extra verbosity (or 
just the hint itself, if one likes the verbosity).

one could even go one step further and put at least the destructive 
commands into interactive/confirmation mode by default. but that's 
probably a bridge too far, as it would be potentially habit-forming in a 
bad way.

regards

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-23 22:19, Oswald Buddenhagen wrote:
> On Mon, Oct 23, 2023 at 12:29:12PM -0700, Jacob Stopak wrote:
>> Those arrows showing how things move only really apply to "simulating"
>> (dry runs) for specific commands like add, restore, rm, commit, stash,
>> etc, so making the --table proposal a default status output would 
>> still
>> miss those scenarios.
>> 
> you're too focused on the status quo of your own tool. :-)
> there is really nothing that would speak against the real commands
> reporting what they just *actually did*. this would seem rather
> helpful for noobs and other insecure users.
> if one really wanted, "you can also use this with --dry-run" could be
> part of the hint that would say how to turn off the extra verbosity
> (or just the hint itself, if one likes the verbosity).

The hint should be about how to turn the tables and verbosity on, not 
how to get rid of it.

> one could even go one step further and put at least the destructive
> commands into interactive/confirmation mode by default. but that's
> probably a bridge too far, as it would be potentially habit-forming in
> a bad way.
> 
> regards

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Oswald Buddenhagen]

On Mon, Oct 23, 2023 at 10:51:31PM +0200, Dragan Simic wrote:
>The hint should be about how to turn the tables and verbosity on, not 
>how to get rid of it.
>
but that's just backwards. a noob shouldn't be bothered with putting the 
tool into noob-friendly mode, neither actually nor "just" 
psychologically. switching to "expert" mode should be the conscious 
effort. and making it opt-in wouldn't even save the experts any 
annoyance, as they need to opt out from the hint anyway.

regards

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-23 23:14, Oswald Buddenhagen wrote:
> On Mon, Oct 23, 2023 at 10:51:31PM +0200, Dragan Simic wrote:
>> The hint should be about how to turn the tables and verbosity on, not 
>> how to get rid of it.
> 
> but that's just backwards. a noob shouldn't be bothered with putting
> the tool into noob-friendly mode, neither actually nor "just"
> psychologically. switching to "expert" mode should be the conscious
> effort. and making it opt-in wouldn't even save the experts any
> annoyance, as they need to opt out from the hint anyway.

Let's focus on non-noobs as well, which could go nuts by having some 
strange tables displayed after updating git.  For the beginners, having 
something configured is already inevitable, e.g. their credentials, so 
they could also turn on the tables if they wanted so.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

On Mon, Oct 23, 2023 at 10:19:47PM +0200, Oswald Buddenhagen wrote:
> On Mon, Oct 23, 2023 at 12:29:12PM -0700, Jacob Stopak wrote:
> > Those arrows showing how things move only really apply to "simulating"
> > (dry runs) for specific commands like add, restore, rm, commit, stash,
> > etc, so making the --table proposal a default status output would still
> > miss those scenarios.
> > 
> you're too focused on the status quo of your own tool. :-)

Ha it's possible. I created git-sim with a very specific use case in mind
so you're right it's probably worth rethinking that while taking into account
Git's other functionality that wasn't in the picture with an external tool.

> there is really nothing that would speak against the real commands reporting
> what they just *actually did*. this would seem rather helpful for noobs and
> other insecure users.

That's true, it would be just as easy to report the results of a command
(and even easier in some cases) than forecasting the result in a dry-run.
The question is how to decide which one? Do you report the results of
certain commands by default while hints are enabled? And only as a dry run
when the --dry-run / -n flag is added? That actually would make sense as
it wouldn't add "special" behavior to the dry run. The dry run would just
report the exact same default output as the normal command, but omit the action.

> if one really wanted, "you can also use this with --dry-run" could be part
> of the hint that would say how to turn off the extra verbosity (or just the
> hint itself, if one likes the verbosity).

Interesting. So many ways to think about how to optimize the user
interaction...

> one could even go one step further and put at least the destructive commands
> into interactive/confirmation mode by default. but that's probably a bridge
> too far, as it would be potentially habit-forming in a bad way.

That would be an interesting add on to our discussion above. So as a
thought experiment, let's pretend there are no restrictions from
traditional users, we could:

  1) Enable verbose results output by default to certain commands, which
     could include a visual table-based output where applicable, and a
     hint to disable it. (Are hints currently command-specific? Or even
     scenario-specific within a command? Or is it all or nothing?)

  2) Include verbose/table results on dry runs, and add dry run flags onto
     commands that should seem to have it for consistency. For example
     "git add" has a dry run flag but "git restore" (the hinted inverse
     operation) does not. I assume on dry runs hints wouldn't be used to
     communicate anything.

  3) Well, I'll admit about once every 3 months I run "git stash --all"
     when I really meant "git stash --include-untracked" and proceed to
     lose a small part of my soul. This would be saved by a simple
     confirmation. I find that the stuff like "git reset --hard" doesn't
     bother me anymore since I know when to be careful with it and what
     things I can get back and how to do it. But I find the nasty ones
     are the things that sound like what you want but end up doing
     something bad. Not sure there is a way to isolate those though...

     I'm rambling now. But maybe for interactivity, at the very least it
     could be added to dry runs, like a "Here's what would happen, want
     to run it now?" I got this feedback a few times for git-sim as well.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-24 01:17, Jacob Stopak wrote:
> That's true, it would be just as easy to report the results of a 
> command
> (and even easier in some cases) than forecasting the result in a 
> dry-run.
> The question is how to decide which one? Do you report the results of
> certain commands by default while hints are enabled? And only as a dry 
> run
> when the --dry-run / -n flag is added? That actually would make sense 
> as
> it wouldn't add "special" behavior to the dry run. The dry run would 
> just
> report the exact same default output as the normal command, but omit 
> the action.

IMHO, it would be the best to simply implement support for 
"<command>.verbose = table" in the git configuration, similarly to how 
we already have "commit.verbose = true".  That way tables could be 
enabled per command, according to the user's preferences, regardless of 
performing dry runs or not.

The new hint would be placed somewhere, which should be decided 
separately, but having the hint enabled or disabled wouldn't affect 
anything else.  Just like with the other hints.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Junio C Hamano]

Dragan Simic <dsimic@manjaro.org> writes:

> IMHO, it would be the best to simply implement support for
> "<command>.verbose = table" in the git configuration, similarly to how
> we already have "commit.verbose = true".  That way tables could be
> enabled per command, according to the user's preferences, regardless
> of performing dry runs or not.

I think the "use more verbose report format to help relatively
inexperienced folks, in exchange for spending more screen real
estate" is a good direction to think about this thing.

I am not personally interested in adding such support all that much
myself, but one piece of advice I can offer those who are interested
is not to be too deeply attached to the word "table".

It may be that "git status" (not "status -s" [*] but the current
version for human consumption) shows "paths with changes to be
committed (i.e. changes added to the index exist)" and "paths with
changes you could add to the index (i.e. changes yet to be added to
the index exist)" in a separate list, and Jacob may have found that
it gives a more understandable view into the states of each path if
the output is turned 90-degrees and the changes are shown in a
tabular form.  But "table" in this example is merely an
implementation detail of one presentation that is easier to
understand for "git status", and calling it "table" and making it a
word in the vocabulary of <command>.verbose is like a tail wagging
the dog.  We want to convey to the users that the option is about
"with extra verbosity, the user is buying a bit more clarity", not
necessarily "use tabular form no matter what".

For some of the commands, tabular presentation might not even be the
presentation form that is the easiest to understand to novices.  For
example, I just pushed out today's integration result to some of my
repositories, and "git push" output looks like so:

To github.com:gitster/git.git
 + 5cb4030332...7dc6f5ada8 ak/color-decorate-symbols -> ak/colo...
 + a71066b71b...c80a646458 jch -> jch (forced update)
 + 89a1ffc6a4...416cdf7260 seen -> seen (forced update)
 + 7ff160463b...2c610ca9ff tb/merge-tree-write-pack -> tb/merge...
   2e3b7b2460..57243409ad  refs/notes/amlog -> refs/notes/amlog

This is already "tabular" and gives enough information to tell me
which ones did not get updated (e.g., I do not see 'next' there) and
which ones are forced ('jch' and 'seen' are usually forced and I'll
notice that I may have made mistakes if they are not).  But a
hypothetical presentation that is easier for novices to read may
show "git log --oneline --graph old...new" (or some abbreviated form
of it) between the old and new tips of the affected branches.  At
that point, calling the improved output as "table" would make little
sense.

For commands that Jacob found it easier to explain in tabular form,
like "git add", it is very possible that two years down the road,
another Jacob comes around and proposes a different presentation
that is even easier for novices to understand, and it may not be a
tabular form.

So be very careful when choosing what to call this new thing, and
avoid naming it after the implementation details (e.g., in what
particular shape the data gets presented) that may turn out not to
be the most important part of the concept.

[Footnote]

 * FWIW, "git status -s" is a tabular presentation.  Maybe we can
   add a more verbose form of "-s" and be done with it for the
   command?

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-24 04:03, Junio C Hamano wrote:
> I think the "use more verbose report format to help relatively
> inexperienced folks, in exchange for spending more screen real
> estate" is a good direction to think about this thing.
> 
> I am not personally interested in adding such support all that much
> myself, but one piece of advice I can offer those who are interested
> is not to be too deeply attached to the word "table".
> 
> ... snip ...
> 
> So be very careful when choosing what to call this new thing, and
> avoid naming it after the implementation details (e.g., in what
> particular shape the data gets presented) that may turn out not to
> be the most important part of the concept.

Totally agreed, "table" simply sneaked in and remained here as the term. 
  Perhaps "<command>.verbose = extra" or something similar would be a 
good choice.

> [Footnote]
> 
>  * FWIW, "git status -s" is a tabular presentation.  Maybe we can
>    add a more verbose form of "-s" and be done with it for the
>    command?

That's also an option.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-24 04:21, Dragan Simic wrote:
> On 2023-10-24 04:03, Junio C Hamano wrote:
>> I think the "use more verbose report format to help relatively
>> inexperienced folks, in exchange for spending more screen real
>> estate" is a good direction to think about this thing.
>> 
>> I am not personally interested in adding such support all that much
>> myself, but one piece of advice I can offer those who are interested
>> is not to be too deeply attached to the word "table".
>> 
>> ... snip ...
>> 
>> So be very careful when choosing what to call this new thing, and
>> avoid naming it after the implementation details (e.g., in what
>> particular shape the data gets presented) that may turn out not to
>> be the most important part of the concept.
> 
> Totally agreed, "table" simply sneaked in and remained here as the
> term.  Perhaps "<command>.verbose = extra" or something similar would
> be a good choice.

Just a brief update...  Some of the associated patches are already ready 
to go, and some more are still pending to be implemented.  It took me a 
while, which I apologize for, and now I've also unfortunately contracted 
some really bad flu.  I'll be back to the patches as soon as the flu 
decides to go away.

>> [Footnote]
>> 
>>  * FWIW, "git status -s" is a tabular presentation.  Maybe we can
>>    add a more verbose form of "-s" and be done with it for the
>>    command?
> 
> That's also an option.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

On Fri, Jan 05, 2024 at 08:14:34PM +0100, Dragan Simic wrote:
> 
> Just a brief update...  Some of the associated patches are already ready to
> go, and some more are still pending to be implemented.  It took me a while,
> which I apologize for, and now I've also unfortunately contracted some
> really bad flu.  I'll be back to the patches as soon as the flu decides to
> go away.

Hey! No worries - thanks for letting me know. I have been thinking about this
and am looking forward to getting back to it.

Hope you recover quickly. Please include me when those patches are ready. Then 
I will try to align my previous patches with them and also I have a bunch of
restructuring/refactoring to do based on the previous feedback from Junio.

Oh and happy new year.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2024-01-06 05:44, Jacob Stopak wrote:
> On Fri, Jan 05, 2024 at 08:14:34PM +0100, Dragan Simic wrote:
>> 
>> Just a brief update...  Some of the associated patches are already 
>> ready to
>> go, and some more are still pending to be implemented.  It took me a 
>> while,
>> which I apologize for, and now I've also unfortunately contracted some
>> really bad flu.  I'll be back to the patches as soon as the flu 
>> decides to
>> go away.
> 
> Hey! No worries - thanks for letting me know. I have been thinking 
> about this
> and am looking forward to getting back to it.
> 
> Hope you recover quickly. Please include me when those patches are 
> ready. Then
> I will try to align my previous patches with them and also I have a 
> bunch of
> restructuring/refactoring to do based on the previous feedback from 
> Junio.

I hope to recover soon.  It's been already about two miserable weeks 
since the flu symptoms started, and it has since moved into my lungs, 
making things much worse than usual.

Sure, I'll also include a brief summary of our earlier discussions into 
the cover letter, with the links to the mailing list archives, to 
provide context for the patches for everyone reviewing them later.  
Looking forward to these new git features!

> Oh and happy new year.

Happy New Year to you too, and to everybody on the mailing list! :)

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-23 21:29, Jacob Stopak wrote:
> However, now that I'm thinking about it maybe it could somehow be 
> included
> in the Hints feature? I honestly don't know exactly when the hints are
> currently invoked or how much detail they go into, but what just popped
> into my head is kindof a "universal dry run" option, which would show 
> the
> user the --table format hint when they invoke an applicable command, 
> and
> prompt them if they actually want to run it.

That's a good idea, having the tables and dry runs mentioned in a 
well-placed hint that could, of course, be disabled through git 
configuration.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Junio C Hamano]

Jacob Stopak <jacob@initialcommit.io> writes:

> Git-Sim is a visual dry-run tool for Git that creates images simulating
> what the corresponding Git command will do, without actually making any
> change to the underlying repo state. Another important aspect is that
> command syntax mimics Git's exactly - so to simulate any Git command, like:

Ah, OK, now I see where your "--table" is coming from ;-).
"git-sim" was exactly what I thought about when I saw it, and I did
not know that "--table" came from the same set of brain cells.

One thing that nobody seems to have raised that disturbs me is that
even though there may be educational value in having such a
"feature", having to carry the extra code to implement in Git incurs
extra cost.  I was reasonably happy when I saw that "git-sim" was
done as a totally separate entity exactly for this reason.

THanks.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-23 21:01, Junio C Hamano wrote:
> Jacob Stopak <jacob@initialcommit.io> writes:
> 
>> Git-Sim is a visual dry-run tool for Git that creates images 
>> simulating
>> what the corresponding Git command will do, without actually making 
>> any
>> change to the underlying repo state. Another important aspect is that
>> command syntax mimics Git's exactly - so to simulate any Git command, 
>> like:
> 
> Ah, OK, now I see where your "--table" is coming from ;-).
> "git-sim" was exactly what I thought about when I saw it, and I did
> not know that "--table" came from the same set of brain cells.
> 
> One thing that nobody seems to have raised that disturbs me is that
> even though there may be educational value in having such a
> "feature", having to carry the extra code to implement in Git incurs
> extra cost.  I was reasonably happy when I saw that "git-sim" was
> done as a totally separate entity exactly for this reason.

That's exactly why I already wrote that the original author of the table 
patches, if those would become accepted, should commit in advance to 
maintaining that as the new git feature.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Oswald Buddenhagen]

>On 2023-10-23 21:01, Junio C Hamano wrote:
>> One thing that nobody seems to have raised that disturbs me is that
>> even though there may be educational value in having such a
>> "feature", having to carry the extra code to implement in Git incurs
>> extra cost.
>
that's the case for literally every feature. you're just noticing it  
here, because from your expert perspective it seems redundant with 
pre-existing functionality. so assuming that you actually care about 
newbie UX, you are at least not convinced that it would be a significant 
help.

i'm not totally convinced either, which is why i wrote of "valid 
experiment" upthread. but i don't think that measuring the actual impact 
is all that interesting unless the feature turns out to be actually 
excessively costly in the long run.

On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
>That's exactly why I already wrote that the original author of the table 
>patches, if those would become accepted, should commit in advance to 
>maintaining that as the new git feature.
>
that's seems just a wee bit unfair (the bar isn't put that high for 
other features), and it's not realistically enforcable anyway.

regards

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

> On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
>> That's exactly why I already wrote that the original author of
>> the table patches, if those would become accepted, should commit
>> in advance to maintaining that as the new git feature.
>> 
> that's seems just a wee bit unfair (the bar isn't put that high for
> other features), and it's not realistically enforcable anyway.

Well, it's a bit disruptive new feature, which is also quite extensive 
and can rather easily become obsolete if not maintained in the long run. 
  Perhaps we should hear Jacob's thoughts about that as well.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

On Mon, Oct 23, 2023 at 10:59:08PM +0200, Dragan Simic wrote:
> > On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
> > > That's exactly why I already wrote that the original author of
> > > the table patches, if those would become accepted, should commit
> > > in advance to maintaining that as the new git feature.
> > > 
> > that's seems just a wee bit unfair (the bar isn't put that high for
> > other features), and it's not realistically enforcable anyway.
> 
> Well, it's a bit disruptive new feature, which is also quite extensive and
> can rather easily become obsolete if not maintained in the long run.
> Perhaps we should hear Jacob's thoughts about that as well.

I would love to work on implementing this and maintaining it! I spend
too much time doing Git things for no reason anyway so why not? :D

But... I could die at the hands of an angry user and then you would have
to take over, Dragan...

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Dragan Simic]

On 2023-10-23 23:23, Jacob Stopak wrote:
> On Mon, Oct 23, 2023 at 10:59:08PM +0200, Dragan Simic wrote:
>> > On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
>> > > That's exactly why I already wrote that the original author of
>> > > the table patches, if those would become accepted, should commit
>> > > in advance to maintaining that as the new git feature.
>> > >
>> > that's seems just a wee bit unfair (the bar isn't put that high for
>> > other features), and it's not realistically enforcable anyway.
>> 
>> Well, it's a bit disruptive new feature, which is also quite extensive 
>> and
>> can rather easily become obsolete if not maintained in the long run.
>> Perhaps we should hear Jacob's thoughts about that as well.
> 
> I would love to work on implementing this and maintaining it! I spend
> too much time doing Git things for no reason anyway so why not? :D

Awesome!  Quite frankly, I expected to hear that. :)

> But... I could die at the hands of an angry user and then you would 
> have
> to take over, Dragan...

Or I could instead try to help you fending off angry git users? :)

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

On Mon, Oct 23, 2023 at 12:01:14PM -0700, Junio C Hamano wrote:
> Ah, OK, now I see where your "--table" is coming from ;-).
> "git-sim" was exactly what I thought about when I saw it, and I did
> not know that "--table" came from the same set of brain cells.

Haha the set is not quite the same I'm sure I've lost many over the
course of this year. But the survivors are doing their best.

> One thing that nobody seems to have raised that disturbs me is that
> even though there may be educational value in having such a
> "feature", having to carry the extra code to implement in Git incurs
> extra cost.  I was reasonably happy when I saw that "git-sim" was
> done as a totally separate entity exactly for this reason.

Erm, not to get too sappy here but I'd love to maintain anything related
to this that gets implemented, in whatever form that turns out to be.

I already spend way too much of my free time working on Git-related
things so making this contribution to Git itself would mean a lot to me.

Starting Git-Sim as a separate entity made sense to me because:

  * I had no idea whether anyone wanted something like this, so it
    would have made for a pretty weak argument to the community. (It
    turned out way more users were interested than I thought).

  * It's written in Python and relies on a dependency library called
    Manim, so I thought it wouldn't make any sense to try and wedge
    that into the Git codebase.

  * The output is presentation quality images / video animations, which
    is unlike anything I've seen outputted by any Git command. (I didn't
    explore what it would take to do something similar in C).

The main downsides to Git-Sim as a separate tool are:

  * The main downside is lack of reach. Not being Git-native means only
    a tiny fraction of Git users will ever know it exists.

  * There is technically no guarantee that a simulated output actually
    corresponds to what the command will do, as highlighted by this
    comment on Hacker News: "Next HN post - "I destroyed my repo - but
    it WORKED in Git-Sim!"

  * As Git changes over time, Git-Sim is destined to be a step behind.

  * Certain commands (like the networked commands fetch/push/pull/etc)
    are not easy to simulate without doing horrible things like cloning
    a new copy of the repo behind the scenes, running the desired
    operation on it, and checking the result. I assume things like this
    would be a lot easier to do within Git.

Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands

[Jacob Stopak]

On Sun, Oct 22, 2023 at 12:30:16PM +0200, Oswald Buddenhagen wrote:
> On Sun, Oct 22, 2023 at 08:38:19AM +0200, Dragan Simic wrote:
> > True, but I still think that having git put its thoughts into tables is
> > actually not helpful.
> > 
> i'm not convinced that the proposed feature specifically would have helped
> me, either (i found the index a rather obvious concept once i knew that it's
> there), but i'm making a general argument here. so:
> 

Thank you for the input! One point I'd like to add is that although the
current proposal patch series only implements the table option for the
status and add commands, it could be applied to many others as dry runs,
as mentioned in my cover letter, some of which touch on concepts besides
the index. Examples would be git stash and git clean. It can take a while
before all these commands feel natural, which was my hope for having this
optional helper when the user simply could use a bit more clarity.

> > To be precise, it actually might be helpful, but only to the first
> > category of users, who will never reach it.  I mean, never say never,
> > but in this case I'm pretty sure it's safe to say it.
> > 
> well, and i think that you're wrong about that.
> your categorization is simply wrong, because it assumes an incorrect static
> model.
> 
> while for the last decade i've been as much of a git expert as one can
> reasonably be without being literally obsessed with it or having written
> much of it, i absolutely *did* start out in your first category (as in, it
> was forced upon me, while i couldn't have cared less about the specifics -
> p4 was working well enough (or so i thought)). and i hated this stupid git
> (it was 2009, and it was much more of a pita for noobs than it is now). i
> certainly could have used more sensible visualizations at every step - on
> the command line, because that's where i mostly "live".
> 

:D. I feel similar and as mentioned in a reply above, the main benefit
to getting direct feedback on in the cli is that it can provide guidance
based on the exact context the user is in, instead of an external
resource which can in most cases only suggest a more general or
tangential guidance / solution.

[RFC PATCH v2 0/6] Noobify format for status, add, restore

[Jacob Stopak]

Take into account reviewer feedback by doing several things differently:

  * Rename this feature (for now) as "noob format mode" (or just "noob
    mode") instead of the original "--table" verbiage. As pointed out,
    this no longer ties the name of the setting to it's proposed
    implementation detail as a table. Noob mode is not necessarily the
    right name, just a placeholder for now. Unless people like it :D

  * Instead of manually having to invoke the -t, --table every time this
    format is to be used, set the config option "status.noob" to true.
    Although this is logically tied to the status command, there are many
    commands that produce status output, (and this series adds more), so
    assume that if the user wants to see the status this way, that it
    should be enabled whenever the status info is displayed.

  * When running "git add" and "git restore" while noob mode is enabled,
    perform the add/restore function as usual, but display the table
    formatted output with arrows showing how file changes moved around.
    Displaying the output in this understandable format after each
    command execution allows the noob to immediately see what they did.
    Although this series only implements for status, add, and restore,
    this output format would make sense in other commands like rm, mv,
    commit, clean, and stash.

  * Works consistently with commands that already have a --dry-run
    (-n) option. The dry run shows the exact same output, but
    doesn't actually do the thing.

  * If `advice.statusHints` is true, add a table footer with status hints.
    Shorten these hints so that they are still clear but better fit into a
    table. Make the hint text yellow to distinguish them. The hints only
    appear when explicitly running "git status", which helps the user
    answer the question "what can I do next?". Hints are omitted in
    "impact" commands like add and restore. Having hints here distracts
    from the file change moves being showed in the table by arrows.

TODO:

  * "git status" outputs myriad other information depending on the state
    of the repo, like branch info, merge conflicts, rebase info, bisect,
    etc. Need to think about how to convey that info with the new setting.

  * Some commands (like stash) might need more than 3 table columns to
    display everything clearly.

  * For destructive commands, think about adding a prompt describing the
    effect, so the user can confirm before the action is taken.

  * Fix horrible things in the patch series code.

  * Probably other things.

Play around with it! It's fun!

Jacob Stopak (6):
  status: add noob format from status.noob config
  status: handle long paths in noob format
  add: implement noob mode
  add: set unique color for noob mode arrows
  restore: implement noob mode
  status: add advice status hints as table footer

 Makefile           |   2 +
 builtin/add.c      |  47 +++++--
 builtin/checkout.c |  46 +++++--
 builtin/commit.c   | 157 +----------------------
 commit.c           |   2 +
 noob.c             | 198 +++++++++++++++++++++++++++++
 noob.h             |  21 ++++
 read-cache-ll.h    |  10 +-
 read-cache.c       |  41 +++++-
 table.c            | 301 +++++++++++++++++++++++++++++++++++++++++++++
 table.h            |   6 +
 wt-status.c        |  75 +++++++----
 wt-status.h        |   6 +
 13 files changed, 708 insertions(+), 204 deletions(-)
 create mode 100644 noob.c
 create mode 100644 noob.h
 create mode 100644 table.c
 create mode 100644 table.h

-- 
2.42.0.404.g2bcc23f3db

[RFC PATCH v2 1/6] status: add noob format from status.noob config

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 Makefile         |   1 +
 builtin/commit.c |   7 +++
 table.c          | 117 +++++++++++++++++++++++++++++++++++++++++++++++
 table.h          |   6 +++
 wt-status.c      |  72 +++++++++++++++++++----------
 wt-status.h      |   1 +
 6 files changed, 179 insertions(+), 25 deletions(-)
 create mode 100644 table.c
 create mode 100644 table.h

diff --git a/Makefile b/Makefile
index 9c6a2f125f..a7399ca8f0 100644
--- a/Makefile
+++ b/Makefile
@@ -1155,6 +1155,7 @@ LIB_OBJS += submodule-config.o
 LIB_OBJS += submodule.o
 LIB_OBJS += symlinks.o
 LIB_OBJS += tag.o
+LIB_OBJS += table.o
 LIB_OBJS += tempfile.o
 LIB_OBJS += thread-utils.o
 LIB_OBJS += tmp-objdir.o
diff --git a/builtin/commit.c b/builtin/commit.c
index 7da5f92448..880c42f5b7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1430,6 +1430,13 @@ static int git_status_config(const char *k, const char *v,
 			status_deferred_config.status_format = STATUS_FORMAT_NONE;
 		return 0;
 	}
+	if (!strcmp(k, "status.noob")) {
+		if (git_config_bool(k, v))
+			status_deferred_config.status_format = STATUS_FORMAT_NOOB;
+		else
+			status_deferred_config.status_format = STATUS_FORMAT_NONE;
+		return 0;
+	}
 	if (!strcmp(k, "status.branch")) {
 		status_deferred_config.show_branch = git_config_bool(k, v);
 		return 0;
diff --git a/table.c b/table.c
new file mode 100644
index 0000000000..15600e117f
--- /dev/null
+++ b/table.c
@@ -0,0 +1,117 @@
+#define USE_THE_INDEX_VARIABLE
+#include "builtin.h"
+#include "gettext.h"
+#include "strbuf.h"
+#include "wt-status.h"
+#include "config.h"
+#include "string-list.h"
+#include "sys/ioctl.h"
+
+static const char *color(int slot, struct wt_status *s)
+{
+	const char *c = "";
+	if (want_color(s->use_color))
+		c = s->color_palette[slot];
+	if (slot == WT_STATUS_ONBRANCH && color_is_nil(c))
+		c = s->color_palette[WT_STATUS_HEADER];
+	return c;
+}
+
+static void build_table_border(struct strbuf *buf, int cols)
+{
+	strbuf_reset(buf);
+	strbuf_addchars(buf, '-', cols);
+}
+
+static void build_table_entry(struct strbuf *buf, char *entry, int cols)
+{
+	strbuf_reset(buf);
+	strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+	strbuf_addstr(buf, entry);
+
+	/* Bump right padding if entry length is odd */
+	if (!(strlen(entry) % 2))
+		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2 + 1);
+	else
+		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+}
+
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+{
+	printf(_("|"));
+	color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
+	printf(_("|"));
+	color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
+	printf(_("|"));
+	color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
+	printf(_("|\n"));
+}
+
+void print_noob_status(struct wt_status *s)
+{
+	struct winsize w;
+	int cols;
+	struct strbuf table_border = STRBUF_INIT;
+	struct strbuf table_col_entry_1 = STRBUF_INIT;
+	struct strbuf table_col_entry_2 = STRBUF_INIT;
+	struct strbuf table_col_entry_3 = STRBUF_INIT;
+	struct string_list_item *item;
+
+	/* Get terminal width */
+	ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
+	cols = w.ws_col;
+
+	/* Ensure table is divisible into 3 even columns */
+	while (((cols - 1) % 3) > 0 || !(cols % 2)) {
+		cols -= 1;
+	}
+
+	build_table_border(&table_border, cols);
+	build_table_entry(&table_col_entry_1, "Untracked files", cols);
+	build_table_entry(&table_col_entry_2, "Unstaged changes", cols);
+	build_table_entry(&table_col_entry_3, "Staging area", cols);
+
+	/* Draw table header */
+	printf(_("%s\n"), table_border.buf);
+	printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+	printf(_("%s\n"), table_border.buf);
+
+	/* Draw table body */
+	for_each_string_list_item(item, &s->untracked) {
+		build_table_entry(&table_col_entry_1, item->string, cols);
+		build_table_entry(&table_col_entry_2, "", cols);
+		build_table_entry(&table_col_entry_3, "", cols);
+		print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+	}
+
+	for_each_string_list_item(item, &s->change) {
+		struct wt_status_change_data *d = item->util;
+		if (d->worktree_status && d->index_status) {
+			build_table_entry(&table_col_entry_1, "", cols);
+			build_table_entry(&table_col_entry_2, item->string, cols);
+			build_table_entry(&table_col_entry_3, item->string, cols);
+		} else if (d->worktree_status) {
+			build_table_entry(&table_col_entry_1, "", cols);
+			build_table_entry(&table_col_entry_2, item->string, cols);
+			build_table_entry(&table_col_entry_3, "", cols);
+		} else if (d->index_status) {
+			build_table_entry(&table_col_entry_1, "", cols);
+			build_table_entry(&table_col_entry_2, "", cols);
+			build_table_entry(&table_col_entry_3, item->string, cols);
+		}
+		print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+	}
+	
+	if (!s->untracked.nr && !s->change.nr) {
+		build_table_entry(&table_col_entry_1, "-", cols);
+		build_table_entry(&table_col_entry_2, "-", cols);
+		build_table_entry(&table_col_entry_3, "-", cols);
+		printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+	}
+
+	printf(_("%s\n"), table_border.buf);
+	strbuf_release(&table_border);
+	strbuf_release(&table_col_entry_1);
+	strbuf_release(&table_col_entry_2);
+	strbuf_release(&table_col_entry_3);
+}
diff --git a/table.h b/table.h
new file mode 100644
index 0000000000..c9e8c386de
--- /dev/null
+++ b/table.h
@@ -0,0 +1,6 @@
+#ifndef TABLE_H
+#define TABLE_H
+
+void print_noob_status(struct wt_status *s);
+
+#endif /* TABLE_H */
diff --git a/wt-status.c b/wt-status.c
index 9f45bf6949..712807aa8f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -31,6 +31,7 @@
 #include "lockfile.h"
 #include "sequencer.h"
 #include "fsmonitor-settings.h"
+#include "table.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
 #define UF_DELAY_WARNING_IN_MS (2 * 1000)
@@ -1833,39 +1834,46 @@ static void wt_longstatus_print_state(struct wt_status *s)
 		show_sparse_checkout_in_use(s, state_color);
 }
 
-static void wt_longstatus_print(struct wt_status *s)
+static void wt_longstatus_print_onwhat(struct wt_status *s, const char *branch_name)
 {
+	const char *on_what = _("On branch ");
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+
+	if (!strcmp(branch_name, "HEAD")) {
+		branch_status_color = color(WT_STATUS_NOBRANCH, s);
+		if (s->state.rebase_in_progress ||
+		    s->state.rebase_interactive_in_progress) {
+			if (s->state.rebase_interactive_in_progress)
+				on_what = _("interactive rebase in progress; onto ");
+			else
+				on_what = _("rebase in progress; onto ");
+			branch_name = s->state.onto;
+		} else if (s->state.detached_from) {
+			branch_name = s->state.detached_from;
+			if (s->state.detached_at)
+				on_what = _("HEAD detached at ");
+			else
+				on_what = _("HEAD detached from ");
+		} else {
+			branch_name = "";
+			on_what = _("Not currently on any branch.");
+		}
+	} else
+		skip_prefix(branch_name, "refs/heads/", &branch_name);
+
+	status_printf_more(s, branch_status_color, "%s", on_what);
+	status_printf_more(s, branch_color, "%s\n", branch_name);
+}
+
+static void wt_longstatus_print(struct wt_status *s)
+{
 	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
-		const char *on_what = _("On branch ");
 		const char *branch_name = s->branch;
-		if (!strcmp(branch_name, "HEAD")) {
-			branch_status_color = color(WT_STATUS_NOBRANCH, s);
-			if (s->state.rebase_in_progress ||
-			    s->state.rebase_interactive_in_progress) {
-				if (s->state.rebase_interactive_in_progress)
-					on_what = _("interactive rebase in progress; onto ");
-				else
-					on_what = _("rebase in progress; onto ");
-				branch_name = s->state.onto;
-			} else if (s->state.detached_from) {
-				branch_name = s->state.detached_from;
-				if (s->state.detached_at)
-					on_what = _("HEAD detached at ");
-				else
-					on_what = _("HEAD detached from ");
-			} else {
-				branch_name = "";
-				on_what = _("Not currently on any branch.");
-			}
-		} else
-			skip_prefix(branch_name, "refs/heads/", &branch_name);
 		status_printf(s, color(WT_STATUS_HEADER, s), "%s", "");
-		status_printf_more(s, branch_status_color, "%s", on_what);
-		status_printf_more(s, branch_color, "%s\n", branch_name);
+		wt_longstatus_print_onwhat(s, branch_name);
 		if (!s->is_initial)
 			wt_longstatus_print_tracking(s);
 	}
@@ -2133,6 +2141,17 @@ static void wt_shortstatus_print(struct wt_status *s)
 		wt_shortstatus_other(it, s, "!!");
 }
 
+static void wt_noobstatus_print(struct wt_status *s)
+{
+	if (s->show_branch) {
+		const char *branch_name = s->branch;
+		wt_longstatus_print_onwhat(s, branch_name);
+		wt_longstatus_print_tracking(s);
+	}
+
+	print_noob_status(s);
+}
+
 static void wt_porcelain_print(struct wt_status *s)
 {
 	s->use_color = 0;
@@ -2560,6 +2579,9 @@ void wt_status_print(struct wt_status *s)
 	case STATUS_FORMAT_LONG:
 		wt_longstatus_print(s);
 		break;
+	case STATUS_FORMAT_NOOB:
+		wt_noobstatus_print(s);
+		break;
 	}
 
 	trace2_region_leave("status", "print", s->repo);
diff --git a/wt-status.h b/wt-status.h
index ab9cc9d8f0..3f08f0d72b 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -73,6 +73,7 @@ enum wt_status_format {
 	STATUS_FORMAT_SHORT,
 	STATUS_FORMAT_PORCELAIN,
 	STATUS_FORMAT_PORCELAIN_V2,
+	STATUS_FORMAT_NOOB,
 
 	STATUS_FORMAT_UNSPECIFIED
 };
-- 
2.42.0.404.g2bcc23f3db

Re: [RFC PATCH v2 1/6] status: add noob format from status.noob config

[Junio C Hamano]

Jacob Stopak <jacob@initialcommit.io> writes:

> diff --git a/table.c b/table.c
> new file mode 100644
> index 0000000000..15600e117f
> --- /dev/null
> +++ b/table.c

Yuck, do we need an entirely new file?  What trait are the things
that are thrown into this file together supposed to share [*]?  It
is not very clear to me what the focus of this file is.

	Side note: for example, stuff in wt-status.c are to compute
	per-path status of the working tree and in-index files.

> @@ -0,0 +1,117 @@
> +#define USE_THE_INDEX_VARIABLE

I personally do not mind, but I suspect many people hate to see this
compatibility set of macros used in a newly written source file.

> +static const char *color(int slot, struct wt_status *s)
> +{
> +	const char *c = "";
> +	if (want_color(s->use_color))
> +		c = s->color_palette[slot];
> +	if (slot == WT_STATUS_ONBRANCH && color_is_nil(c))
> +		c = s->color_palette[WT_STATUS_HEADER];
> +	return c;
> +}

Do we need to duplicate this from other files?  If this is about
"git status", perhaps some parts of this patch, the truly new things
(rather than what was copied, like this one) can be added to
wt-status.c instead of adding a new file with unclear focus?

> +static void build_table_border(struct strbuf *buf, int cols)
> +{
> +	strbuf_reset(buf);
> +	strbuf_addchars(buf, '-', cols);
> +}

This seems to be horizontal border; do we need a separate vertical
border?

> +static void build_table_entry(struct strbuf *buf, char *entry, int cols)
> +{
> +	strbuf_reset(buf);
> +	strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
> +	strbuf_addstr(buf, entry);
> +
> +	/* Bump right padding if entry length is odd */
> +	if (!(strlen(entry) % 2))
> +		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2 + 1);
> +	else
> +		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
> +}

The code assumes that one byte in string "entry" occupies one and
only one display columns, which is so 20th centry assumption that
does not care about i18n.  Often what takes 3 bytes in a UTF-8
string occupies 2 display columns, for example.  In addition, if you
plan to color entries in the table, some substring would end up to
be 0-width.  Your pathname may be so long that 1/3 of a window
width may not be sufficient to show it in its entirety, you might
need to show it truncated in the middle.

utf8.c has support for measuring the display width of UTF-8 string,
which is used elsewhere in our code.  You may want to study it if
you want to do a "tabular" output.  The code in diff.c that shows
diffstat has many gems to help what this code wants to do, including
measuring display columns of a string, chomping a long string to fit
in a desired display columns, etc., by using helpers defined in
utf8.c

A potential excuse I can think of to have these outside wt-status.c
and in a separate new file is to have a generic "table" layout
machinery that is independent from "git status" or what each column
of the table is showing (in other words, they may not be pathnames),
and reusable by other subcommands that want to show things in the
"table" layout.  But even as a candidate for such a generic table
mechanism, the above falls far short by hardcoding that it can only
show 3-col table whose columns are evenly distributed and nothing
else.

> +static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
> +{
> +	printf(_("|"));
> +	color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
> +	printf(_("|"));
> +	color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
> +	printf(_("|"));
> +	color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
> +	printf(_("|\n"));
> +}

How does the code deal with unknown display width of translated
version of "|" emitted here?  Are you assuming that no matter how
these are translated, they will always occupy one display column
each?

> +void print_noob_status(struct wt_status *s)
> +{
> +	struct winsize w;
> +	int cols;
> +	struct strbuf table_border = STRBUF_INIT;
> +	struct strbuf table_col_entry_1 = STRBUF_INIT;
> +	struct strbuf table_col_entry_2 = STRBUF_INIT;
> +	struct strbuf table_col_entry_3 = STRBUF_INIT;
> +	struct string_list_item *item;
> +
> +	/* Get terminal width */
> +	ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
> +	cols = w.ws_col;

Let's not reinvent an incomplete solution before studying and
finding out what we already have in our codebase.  Immediately after
you got tempted to type TIOCGWINSZ, you can "git grep" the codebase
for that particular constant to see if we already use it, as that is
one very reasonable way to achieve what this piece of code wants to
do (i.e. find out what the display width would be).  You'd find
pager.c:term_columns() and also learned that we want to prepare for
the case where the ioctl() is not available.

> +	build_table_entry(&table_col_entry_1, "Untracked files", cols);
> +	build_table_entry(&table_col_entry_2, "Unstaged changes", cols);
> +	build_table_entry(&table_col_entry_3, "Staging area", cols);

Shouldn't these three strings be translatable?

What shoudl happen when these labels are wider than cols/3?

> diff --git a/table.h b/table.h
> new file mode 100644
> index 0000000000..c9e8c386de
> --- /dev/null
> +++ b/table.h
> @@ -0,0 +1,6 @@
> +#ifndef TABLE_H
> +#define TABLE_H
> +
> +void print_noob_status(struct wt_status *s);
> +
> +#endif /* TABLE_H */

I am guessing that your plan is to add other "distim_noob_add()" and
other "noob" variant of operations for various Git subcommands here,
but I really do not think you want to add table.[ch] that has logic
for such random set of Git subcommands copied and tweaked from all
over the place, as the only trait being shared among them will
become "they are written by Jacob Stopak", that is not a very useful
grouping of the functions.  It is not even "this file collects all
the code that produce tabular output from Git"---"git status -s"
already gives tabular output, for example, without using any of the
"I only want to draw a table with three columns of equal width"
logic.  Adding code that are necessary to add yet another output
mode for "git status" directly to where various output modes of "git
status" are implemented, i.e. wt-status.c, and do similar changes for
each command would make more sense, I would think.

Thanks.

Re: [RFC PATCH v2 1/6] status: add noob format from status.noob config

[Dragan Simic]

On 2023-10-30 02:32, Junio C Hamano wrote:
> Jacob Stopak <jacob@initialcommit.io> writes:
>> diff --git a/table.h b/table.h
>> new file mode 100644
>> index 0000000000..c9e8c386de
>> --- /dev/null
>> +++ b/table.h
>> @@ -0,0 +1,6 @@
>> +#ifndef TABLE_H
>> +#define TABLE_H
>> +
>> +void print_noob_status(struct wt_status *s);
>> +
>> +#endif /* TABLE_H */
> 
> I am guessing that your plan is to add other "distim_noob_add()" and
> other "noob" variant of operations for various Git subcommands here,
> but I really do not think you want to add table.[ch] that has logic
> for such random set of Git subcommands copied and tweaked from all
> over the place, as the only trait being shared among them will
> become "they are written by Jacob Stopak", that is not a very useful
> grouping of the functions.  It is not even "this file collects all
> the code that produce tabular output from Git"---"git status -s"
> already gives tabular output, for example, without using any of the
> "I only want to draw a table with three columns of equal width"
> logic.  Adding code that are necessary to add yet another output
> mode for "git status" directly to where various output modes of "git
> status" are implemented, i.e. wt-status.c, and do similar changes for
> each command would make more sense, I would think.

Furthermore, "extended" should perhaps be used instead of "noob" 
throughout, to reflect the planned naming of the configuration option 
values.

Re: [RFC PATCH v2 1/6] status: add noob format from status.noob config

[Jacob Stopak]

On Mon, Oct 30, 2023 at 10:32:57AM +0900, Junio C Hamano wrote:
> Jacob Stopak <jacob@initialcommit.io> writes:
> 
> > diff --git a/table.c b/table.c
> > new file mode 100644
> > index 0000000000..15600e117f
> > --- /dev/null
> > +++ b/table.c
> 
> Yuck, do we need an entirely new file?  What trait are the things
> that are thrown into this file together supposed to share [*]?  It
> is not very clear to me what the focus of this file is.
> 
> 	Side note: for example, stuff in wt-status.c are to compute
> 	per-path status of the working tree and in-index files.

I created a new file because:

  * It will be used by more than just the status command (as patches 3/6
    and 5/6 show it applied to the "add" and "restore" commands
    respectively). And it will be applied to more commands as well.

  * For these RFC versions I mainly wanted to get some kind of minimal
    POC working, so I could get feedback and build on

  * It was easier to work on it in it's own file.

That being said, I was probably careless with what I tossed in there.
I intended it to specifically be things related to populating, coloring,
and printing the new table format.

But after reading all of your comments here, I see that I most likely
made the error (which I've made before) of being too specific. If I
generalize this file to be a generic table format that can be used for
anything, then the command-specific details can likely be extracted into
their respective existing files.

Another thing I totally didn't consider until I read your mail is the
possibility of refactoring the existing "git column" functionality to
provide a configurable table output format. At this point I'm not sure
if that's feasible or makes sense, but it's at least worth considering.

> > @@ -0,0 +1,117 @@
> > +#define USE_THE_INDEX_VARIABLE
> 
> I personally do not mind, but I suspect many people hate to see this
> compatibility set of macros used in a newly written source file.

Noted, I'll try to update this.

> > +static const char *color(int slot, struct wt_status *s)
> > +{
> > +	const char *c = "";
> > +	if (want_color(s->use_color))
> > +		c = s->color_palette[slot];
> > +	if (slot == WT_STATUS_ONBRANCH && color_is_nil(c))
> > +		c = s->color_palette[WT_STATUS_HEADER];
> > +	return c;
> > +}
> 
> Do we need to duplicate this from other files?  If this is about
> "git status", perhaps some parts of this patch, the truly new things
> (rather than what was copied, like this one) can be added to
> wt-status.c instead of adding a new file with unclear focus?

I just re-used the status coloring out of laziness, since I was thinking
about this in a narrow way that it's basically replicating the output
of Git status, so I figured I'd use the same colors.

Now that I'm thinking more generally I see why this needs to change,
to make table column output colors flexible.

> > +static void build_table_border(struct strbuf *buf, int cols)
> > +{
> > +	strbuf_reset(buf);
> > +	strbuf_addchars(buf, '-', cols);
> > +}
> 
> This seems to be horizontal border; do we need a separate vertical
> border?

Yes it is a horizontal border, the vertical border pipe characters are
included as a part of each table line being drawn, so I didn't need a
standalone vertical border in order to "draw" the tables. This one could
be renamed to reflect it's horizontal-ness, for clarity.

> > +static void build_table_entry(struct strbuf *buf, char *entry, int cols)
> > +{
> > +	strbuf_reset(buf);
> > +	strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
> > +	strbuf_addstr(buf, entry);
> > +
> > +	/* Bump right padding if entry length is odd */
> > +	if (!(strlen(entry) % 2))
> > +		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2 + 1);
> > +	else
> > +		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
> > +}
> 
> The code assumes that one byte in string "entry" occupies one and
> only one display columns, which is so 20th centry assumption that
> does not care about i18n.  Often what takes 3 bytes in a UTF-8
> string occupies 2 display columns, for example.  In addition, if you
> plan to color entries in the table, some substring would end up to
> be 0-width.

Thanks for this info. I'll look into these.

> Your pathname may be so long that 1/3 of a window
> width may not be sufficient to show it in its entirety, you might
> need to show it truncated in the middle.

This is exactly what patch 2/6 does (more details on that below). I
should have squashed that into 1/6. I'll do that in the next series.

> utf8.c has support for measuring the display width of UTF-8 string,
> which is used elsewhere in our code.  You may want to study it if
> you want to do a "tabular" output.  The code in diff.c that shows
> diffstat has many gems to help what this code wants to do, including
> measuring display columns of a string, chomping a long string to fit
> in a desired display columns, etc., by using helpers defined in
> utf8.c

Well that sounds like exactly what I need. I'll take a look, thanks!

> A potential excuse I can think of to have these outside wt-status.c
> and in a separate new file is to have a generic "table" layout
> machinery that is independent from "git status" or what each column
> of the table is showing (in other words, they may not be pathnames),
> and reusable by other subcommands that want to show things in the
> "table" layout.  

Yes that is my excuse (altho it may not be a valid one) - this patch
series adds the table output format for the "git add" (see patch 3/6) and
"git restore" (see patch 5/6) commands. Dragan will be working on
implementing verbose config options for several commands, that I will then
use as stepping stones to try and add this table format as the "extended"
(not "noob") verbosity option.

> But even as a candidate for such a generic table
> mechanism, the above falls far short by hardcoding that it can only
> show 3-col table whose columns are evenly distributed and nothing
> else.

Yeah - I was planning to update the table format so that the number of
columns is flexible. For example, I can think of commands like "git
stash" that would likely require 4 columns to display the required info,
so parameterizing the number of columns is on my agenda.

But you're right maybe there are many other useful things that a generic
table format would include. Things like alignment, spacing, etc. I didn't
think about how this might be used outside of my specific use case - I
tend to get tunnel vision on what I'm working on. I'll try to think more
broadly about how a format like this might be applicable, and at least
try to implement it in a way that it can be re-used for other things if
and when the need arises.

> > +static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
> > +{
> > +	printf(_("|"));
> > +	color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
> > +	printf(_("|"));
> > +	color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
> > +	printf(_("|"));
> > +	color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
> > +	printf(_("|\n"));
> > +}
> 
> How does the code deal with unknown display width of translated
> version of "|" emitted here?  Are you assuming that no matter how
> these are translated, they will always occupy one display column
> each?

I'm not familiar with how the translation process works yet, so _if_ the
version of the "|" that I'm emitting here is a canditate for translation,
it was not even my intention to do so, as I'm just using the vertical
pipe as a column separator and table border. I think what I need is to
make sure these are _not_ translatable, so that we can guarantee the
single character display width? 

> > +void print_noob_status(struct wt_status *s)
> > +{
> > +	struct winsize w;
> > +	int cols;
> > +	struct strbuf table_border = STRBUF_INIT;
> > +	struct strbuf table_col_entry_1 = STRBUF_INIT;
> > +	struct strbuf table_col_entry_2 = STRBUF_INIT;
> > +	struct strbuf table_col_entry_3 = STRBUF_INIT;
> > +	struct string_list_item *item;
> > +
> > +	/* Get terminal width */
> > +	ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
> > +	cols = w.ws_col;
> 
> Let's not reinvent an incomplete solution before studying and
> finding out what we already have in our codebase.  Immediately after
> you got tempted to type TIOCGWINSZ, you can "git grep" the codebase
> for that particular constant to see if we already use it, as that is
> one very reasonable way to achieve what this piece of code wants to
> do (i.e. find out what the display width would be).  You'd find
> pager.c:term_columns() and also learned that we want to prepare for
> the case where the ioctl() is not available.

Ah, ok let me look into that. I have utilized git grepping to understand
how various things are done in the codebase, but for some (no good) reason
I didn't think to try that here.

> > +	build_table_entry(&table_col_entry_1, "Untracked files", cols);
> > +	build_table_entry(&table_col_entry_2, "Unstaged changes", cols);
> > +	build_table_entry(&table_col_entry_3, "Staging area", cols);
> 
> Shouldn't these three strings be translatable?

Yes. I'll look into how translatable strings are done in the codebase.

> What should happen when these labels are wider than cols/3?

That's what patch 2/6 does. It splices out the center of strings that
are too long to fit in a column, keeping an equal number of characters
at the start and end of the string, and inserts a "..." in the middle.
Altho this makes sense for paths, this could be a bit confusing for
table headers, since it may be hard (or impossible) to read what the
column's purpose is, so it might be wiser to come up with even shorter
versions of the table headers to use when the console width is below a
certain size that makes the original strings unreadable.

> > diff --git a/table.h b/table.h
> > new file mode 100644
> > index 0000000000..c9e8c386de
> > --- /dev/null
> > +++ b/table.h
> > @@ -0,0 +1,6 @@
> > +#ifndef TABLE_H
> > +#define TABLE_H
> > +
> > +void print_noob_status(struct wt_status *s);
> > +
> > +#endif /* TABLE_H */
> 
> I am guessing that your plan is to add other "distim_noob_add()" and
> other "noob" variant of operations for various Git subcommands here,
> but I really do not think you want to add table.[ch] that has logic
> for such random set of Git subcommands copied and tweaked from all
> over the place, as the only trait being shared among them will
> become "they are written by Jacob Stopak", that is not a very useful
> grouping of the functions.  It is not even "this file collects all
> the code that produce tabular output from Git"---"git status -s"
> already gives tabular output, for example, without using any of the
> "I only want to draw a table with three columns of equal width"
> logic. Adding code that are necessary to add yet another output
> mode for "git status" directly to where various output modes of "git
> status" are implemented, i.e. wt-status.c, and do similar changes for
> each command would make more sense, I would think.

If you look at patches 3/6 and 5/6 I think you'll better understand my
"plan", not that I thought this would by any kind of final version, but
more of a starting point to get feedback and build on (hence the RFC).

I think the reason I clung so hard to "status" is that it contains a
lot of the info that I planned to display across various commands, so
much so that I baked it into the new table file itself. I see now that
needs to be separated out, as that data to be displayed can be calculated
in each command itself, and passed into the table functions to build and
print the thing.

But based on your input in this mail I'm going to:

  * Look into the existing "column" formatter to see if it can be
    repurposed for this table stuff.

  * If not, generalize the table stuff so that it can be used for anything
    and compartmentalize the command-specific stuff where it belongs.

> Thanks.

Thank you!

[RFC PATCH v2 2/6] status: handle long paths in noob format

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 table.c     | 58 ++++++++++++++++++++++++++++++++++++++++++++++-------
 table.h     |  2 +-
 wt-status.c |  2 +-
 3 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/table.c b/table.c
index 15600e117f..d085f2a098 100644
--- a/table.c
+++ b/table.c
@@ -25,15 +25,44 @@ static void build_table_border(struct strbuf *buf, int cols)
 
 static void build_table_entry(struct strbuf *buf, char *entry, int cols)
 {
+	int len = strlen(entry);
+	size_t col_width = (cols / 3) - 5; /* subtract for padding */
+
 	strbuf_reset(buf);
-	strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+
+	/* Trim equally from both sides if it doesn't fit in column */
+	if (len > col_width) {
+		struct strbuf start_buf = STRBUF_INIT;
+		struct strbuf end_buf = STRBUF_INIT;
+		struct strbuf entry_buf = STRBUF_INIT;
+
+		strbuf_addstr(&start_buf, entry);
+		strbuf_addstr(&end_buf, entry);
+
+		strbuf_remove(&start_buf, col_width / 2, len - col_width / 2);
+		strbuf_remove(&end_buf, 0, len - col_width / 2);
+
+		strbuf_addstr(&entry_buf, start_buf.buf);
+		strbuf_addstr(&entry_buf, "...");
+		strbuf_addstr(&entry_buf, end_buf.buf);
+
+		entry = strbuf_detach(&entry_buf, &col_width);
+		len = strlen(entry);
+
+		strbuf_release(&start_buf);
+		strbuf_release(&end_buf);
+		strbuf_release(&entry_buf);
+	}
+
+	strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2); /* left padding */
 	strbuf_addstr(buf, entry);
 
-	/* Bump right padding if entry length is odd */
-	if (!(strlen(entry) % 2))
-		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2 + 1);
+	/* right padding */
+	if (!(len % 2))
+		/* Bump right padding if entry length is odd */
+		strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2 + 1);
 	else
-		strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+		strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
 }
 
 static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
@@ -47,7 +76,7 @@ static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, stru
 	printf(_("|\n"));
 }
 
-void print_noob_status(struct wt_status *s)
+void print_noob_status(struct wt_status *s, int add_advice)
 {
 	struct winsize w;
 	int cols;
@@ -66,14 +95,29 @@ void print_noob_status(struct wt_status *s)
 		cols -= 1;
 	}
 
+	/* Draw table header */
 	build_table_border(&table_border, cols);
 	build_table_entry(&table_col_entry_1, "Untracked files", cols);
 	build_table_entry(&table_col_entry_2, "Unstaged changes", cols);
 	build_table_entry(&table_col_entry_3, "Staging area", cols);
 
-	/* Draw table header */
 	printf(_("%s\n"), table_border.buf);
 	printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+
+	if (add_advice) {
+		build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
+		build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
+		build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
+
+		printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+
+		build_table_entry(&table_col_entry_1, "", cols);
+		build_table_entry(&table_col_entry_2, "(discard: git restore --staged <file>)", cols);
+		build_table_entry(&table_col_entry_3, "", cols);
+
+		printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+	}
+
 	printf(_("%s\n"), table_border.buf);
 
 	/* Draw table body */
diff --git a/table.h b/table.h
index c9e8c386de..5dff7162a4 100644
--- a/table.h
+++ b/table.h
@@ -1,6 +1,6 @@
 #ifndef TABLE_H
 #define TABLE_H
 
-void print_noob_status(struct wt_status *s);
+void print_noob_status(struct wt_status *s, int i);
 
 #endif /* TABLE_H */
diff --git a/wt-status.c b/wt-status.c
index 712807aa8f..b5899dcc98 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2149,7 +2149,7 @@ static void wt_noobstatus_print(struct wt_status *s)
 		wt_longstatus_print_tracking(s);
 	}
 
-	print_noob_status(s);
+	print_noob_status(s, 0);
 }
 
 static void wt_porcelain_print(struct wt_status *s)
-- 
2.42.0.404.g2bcc23f3db

[RFC PATCH v2 3/6] add: implement noob mode

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 Makefile         |   1 +
 builtin/add.c    |  47 ++++++++---
 builtin/commit.c | 163 +-------------------------------------
 commit.c         |   2 +
 noob.c           | 198 +++++++++++++++++++++++++++++++++++++++++++++++
 noob.h           |  21 +++++
 read-cache-ll.h  |   9 ++-
 read-cache.c     |  32 ++++++--
 table.c          |  92 +++++++++++++++++++---
 wt-status.c      |   1 +
 wt-status.h      |   1 +
 11 files changed, 381 insertions(+), 186 deletions(-)
 create mode 100644 noob.c
 create mode 100644 noob.h

diff --git a/Makefile b/Makefile
index a7399ca8f0..78acfaf14d 100644
--- a/Makefile
+++ b/Makefile
@@ -1070,6 +1070,7 @@ LIB_OBJS += name-hash.o
 LIB_OBJS += negotiator/default.o
 LIB_OBJS += negotiator/noop.o
 LIB_OBJS += negotiator/skipping.o
+LIB_OBJS += noob.o
 LIB_OBJS += notes-cache.o
 LIB_OBJS += notes-merge.o
 LIB_OBJS += notes-utils.o
diff --git a/builtin/add.c b/builtin/add.c
index c27254a5cd..dbb99d179e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -27,6 +27,9 @@
 #include "strvec.h"
 #include "submodule.h"
 #include "add-interactive.h"
+#include "wt-status.h"
+#include "commit.h"
+#include "noob.h"
 
 static const char * const builtin_add_usage[] = {
 	N_("git add [<options>] [--] <pathspec>..."),
@@ -322,7 +325,7 @@ static void check_embedded_repo(const char *path)
 	strbuf_release(&name);
 }
 
-static int add_files(struct dir_struct *dir, int flags)
+static int add_files(struct dir_struct *dir, int flags, struct wt_status *status)
 {
 	int i, exit_status = 0;
 	struct string_list matched_sparse_paths = STRING_LIST_INIT_NODUP;
@@ -345,7 +348,7 @@ static int add_files(struct dir_struct *dir, int flags)
 					   dir->entries[i]->name);
 			continue;
 		}
-		if (add_file_to_index(&the_index, dir->entries[i]->name, flags)) {
+		if (add_file_to_index_with_status(&the_index, dir->entries[i]->name, flags, status)) {
 			if (!ignore_add_errors)
 				die(_("adding files failed"));
 			exit_status = 1;
@@ -374,8 +377,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int require_pathspec;
 	char *seen = NULL;
 	struct lock_file lock_file = LOCK_INIT;
-
+	struct wt_status status;
+	unsigned int progress_flag = 0;
+	
+	wt_status_prepare(the_repository, &status);
 	git_config(add_config, NULL);
+	git_config(git_status_config, &status);
+	finalize_deferred_config(&status);
+	status.status_format = status_format;
 
 	argc = parse_options(argc, argv, prefix, builtin_add_options,
 			  builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
@@ -459,7 +468,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		 (intent_to_add ? ADD_CACHE_INTENT : 0) |
 		 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
 		 (!(addremove || take_worktree_changes)
-		  ? ADD_CACHE_IGNORE_REMOVAL : 0));
+		  ? ADD_CACHE_IGNORE_REMOVAL : 0) |
+		 (status.status_format == STATUS_FORMAT_NOOB ? ADD_CACHE_FORMAT_NOOB : 0));
 
 	if (repo_read_index_preload(the_repository, &pathspec, 0) < 0)
 		die(_("index file corrupt"));
@@ -551,15 +561,32 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	begin_odb_transaction();
 
-	if (add_renormalize)
+	if (status.status_format == STATUS_FORMAT_NOOB) {
+		/* Read index and populate status */
+		repo_read_index(the_repository);
+		refresh_index(&the_index,
+			      REFRESH_QUIET|REFRESH_UNMERGED|progress_flag,
+			      &status.pathspec, NULL, NULL);
+		status.show_branch = 0;
+		wt_status_collect(&status);
+	}
+
+	if (add_renormalize) {
 		exit_status |= renormalize_tracked_files(&pathspec, flags);
-	else
-		exit_status |= add_files_to_cache(the_repository, prefix,
+	} else {
+		exit_status |= add_files_to_cache_with_status(the_repository, prefix,
 						  &pathspec, include_sparse,
-						  flags);
+						  flags, &status);
+	}
 
-	if (add_new_files)
-		exit_status |= add_files(&dir, flags);
+	if (add_new_files) {
+		exit_status |= add_files(&dir, flags, &status);
+	}
+
+	if (status.status_format == STATUS_FORMAT_NOOB) {
+		wt_status_print(&status);
+		wt_status_collect_free_buffers(&status);
+	}
 
 	if (chmod_arg && pathspec.nr)
 		exit_status |= chmod_pathspec(&pathspec, chmod_arg[0], show_only);
diff --git a/builtin/commit.c b/builtin/commit.c
index 880c42f5b7..3f816c117d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -46,6 +46,7 @@
 #include "commit-reach.h"
 #include "commit-graph.h"
 #include "pretty.h"
+#include "noob.h"
 
 static const char * const builtin_commit_usage[] = {
 	N_("git commit [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]\n"
@@ -148,8 +149,6 @@ static int use_editor = 1, include_status = 1;
 static int have_option_m;
 static struct strbuf message = STRBUF_INIT;
 
-static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
-
 static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
 {
 	BUG_ON_OPT_NEG(unset);
@@ -310,7 +309,7 @@ static void add_remove_files(struct string_list *list)
 			continue;
 
 		if (!lstat(p->string, &st)) {
-			if (add_to_index(&the_index, p->string, &st, 0))
+			if (add_file_to_index(&the_index, p->string, 0))
 				die(_("updating files failed"));
 		} else
 			remove_file_from_index(&the_index, p->string);
@@ -1196,59 +1195,6 @@ static const char *read_commit_message(const char *name)
 	return repo_logmsg_reencode(the_repository, commit, NULL, out_enc);
 }
 
-/*
- * Enumerate what needs to be propagated when --porcelain
- * is not in effect here.
- */
-static struct status_deferred_config {
-	enum wt_status_format status_format;
-	int show_branch;
-	enum ahead_behind_flags ahead_behind;
-} status_deferred_config = {
-	STATUS_FORMAT_UNSPECIFIED,
-	-1, /* unspecified */
-	AHEAD_BEHIND_UNSPECIFIED,
-};
-
-static void finalize_deferred_config(struct wt_status *s)
-{
-	int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN &&
-				   status_format != STATUS_FORMAT_PORCELAIN_V2 &&
-				   !s->null_termination);
-
-	if (s->null_termination) {
-		if (status_format == STATUS_FORMAT_NONE ||
-		    status_format == STATUS_FORMAT_UNSPECIFIED)
-			status_format = STATUS_FORMAT_PORCELAIN;
-		else if (status_format == STATUS_FORMAT_LONG)
-			die(_("options '%s' and '%s' cannot be used together"), "--long", "-z");
-	}
-
-	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;
-
-	if (use_deferred_config && s->show_branch < 0)
-		s->show_branch = status_deferred_config.show_branch;
-	if (s->show_branch < 0)
-		s->show_branch = 0;
-
-	/*
-	 * If the user did not give a "--[no]-ahead-behind" command
-	 * line argument *AND* we will print in a human-readable format
-	 * (short, long etc.) then we inherit from the status.aheadbehind
-	 * config setting.  In all other cases (and porcelain V[12] formats
-	 * in particular), we inherit _FULL for backwards compatibility.
-	 */
-	if (use_deferred_config &&
-	    s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
-		s->ahead_behind_flags = status_deferred_config.ahead_behind;
-
-	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
-		s->ahead_behind_flags = AHEAD_BEHIND_FULL;
-}
-
 static void check_fixup_reword_options(int argc, const char *argv[]) {
 	if (whence != FROM_COMMIT) {
 		if (whence == FROM_MERGE)
@@ -1399,111 +1345,6 @@ static int dry_run_commit(const char **argv, const char *prefix,
 
 define_list_config_array_extra(color_status_slots, {"added"});
 
-static int parse_status_slot(const char *slot)
-{
-	if (!strcasecmp(slot, "added"))
-		return WT_STATUS_UPDATED;
-
-	return LOOKUP_CONFIG(color_status_slots, slot);
-}
-
-static int git_status_config(const char *k, const char *v,
-			     const struct config_context *ctx, void *cb)
-{
-	struct wt_status *s = cb;
-	const char *slot_name;
-
-	if (starts_with(k, "column."))
-		return git_column_config(k, v, "status", &s->colopts);
-	if (!strcmp(k, "status.submodulesummary")) {
-		int is_bool;
-		s->submodule_summary = git_config_bool_or_int(k, v, ctx->kvi,
-							      &is_bool);
-		if (is_bool && s->submodule_summary)
-			s->submodule_summary = -1;
-		return 0;
-	}
-	if (!strcmp(k, "status.short")) {
-		if (git_config_bool(k, v))
-			status_deferred_config.status_format = STATUS_FORMAT_SHORT;
-		else
-			status_deferred_config.status_format = STATUS_FORMAT_NONE;
-		return 0;
-	}
-	if (!strcmp(k, "status.noob")) {
-		if (git_config_bool(k, v))
-			status_deferred_config.status_format = STATUS_FORMAT_NOOB;
-		else
-			status_deferred_config.status_format = STATUS_FORMAT_NONE;
-		return 0;
-	}
-	if (!strcmp(k, "status.branch")) {
-		status_deferred_config.show_branch = git_config_bool(k, v);
-		return 0;
-	}
-	if (!strcmp(k, "status.aheadbehind")) {
-		status_deferred_config.ahead_behind = git_config_bool(k, v);
-		return 0;
-	}
-	if (!strcmp(k, "status.showstash")) {
-		s->show_stash = git_config_bool(k, v);
-		return 0;
-	}
-	if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
-		s->use_color = git_config_colorbool(k, v);
-		return 0;
-	}
-	if (!strcmp(k, "status.displaycommentprefix")) {
-		s->display_comment_prefix = git_config_bool(k, v);
-		return 0;
-	}
-	if (skip_prefix(k, "status.color.", &slot_name) ||
-	    skip_prefix(k, "color.status.", &slot_name)) {
-		int slot = parse_status_slot(slot_name);
-		if (slot < 0)
-			return 0;
-		if (!v)
-			return config_error_nonbool(k);
-		return color_parse(v, s->color_palette[slot]);
-	}
-	if (!strcmp(k, "status.relativepaths")) {
-		s->relative_paths = git_config_bool(k, v);
-		return 0;
-	}
-	if (!strcmp(k, "status.showuntrackedfiles")) {
-		if (!v)
-			return config_error_nonbool(k);
-		else if (!strcmp(v, "no"))
-			s->show_untracked_files = SHOW_NO_UNTRACKED_FILES;
-		else if (!strcmp(v, "normal"))
-			s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
-		else if (!strcmp(v, "all"))
-			s->show_untracked_files = SHOW_ALL_UNTRACKED_FILES;
-		else
-			return error(_("Invalid untracked files mode '%s'"), v);
-		return 0;
-	}
-	if (!strcmp(k, "diff.renamelimit")) {
-		if (s->rename_limit == -1)
-			s->rename_limit = git_config_int(k, v, ctx->kvi);
-		return 0;
-	}
-	if (!strcmp(k, "status.renamelimit")) {
-		s->rename_limit = git_config_int(k, v, ctx->kvi);
-		return 0;
-	}
-	if (!strcmp(k, "diff.renames")) {
-		if (s->detect_rename == -1)
-			s->detect_rename = git_config_rename(k, v);
-		return 0;
-	}
-	if (!strcmp(k, "status.renames")) {
-		s->detect_rename = git_config_rename(k, v);
-		return 0;
-	}
-	return git_diff_ui_config(k, v, ctx, NULL);
-}
-
 int cmd_status(int argc, const char **argv, const char *prefix)
 {
 	static int no_renames = -1;
diff --git a/commit.c b/commit.c
index b3223478bc..c08faf48fd 100644
--- a/commit.c
+++ b/commit.c
@@ -28,6 +28,8 @@
 #include "shallow.h"
 #include "tree.h"
 #include "hook.h"
+#include "column.h"
+#include "config.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
 
diff --git a/noob.c b/noob.c
new file mode 100644
index 0000000000..680d461698
--- /dev/null
+++ b/noob.c
@@ -0,0 +1,198 @@
+#include "git-compat-util.h"
+#include "tag.h"
+#include "commit.h"
+#include "commit-graph.h"
+#include "environment.h"
+#include "gettext.h"
+#include "hex.h"
+#include "repository.h"
+#include "object-name.h"
+#include "object-store-ll.h"
+#include "pkt-line.h"
+#include "utf8.h"
+#include "diff.h"
+#include "revision.h"
+#include "notes.h"
+#include "alloc.h"
+#include "gpg-interface.h"
+#include "mergesort.h"
+#include "commit-slab.h"
+#include "prio-queue.h"
+#include "hash-lookup.h"
+#include "wt-status.h"
+#include "advice.h"
+#include "refs.h"
+#include "commit-reach.h"
+#include "run-command.h"
+#include "setup.h"
+#include "shallow.h"
+#include "tree.h"
+#include "hook.h"
+#include "column.h"
+#include "config.h"
+#include "noob.h"
+
+static const char *color_status_slots[] = {
+	[WT_STATUS_HEADER]	  = "header",
+	[WT_STATUS_UPDATED]	  = "updated",
+	[WT_STATUS_CHANGED]	  = "changed",
+	[WT_STATUS_UNTRACKED]	  = "untracked",
+	[WT_STATUS_NOBRANCH]	  = "noBranch",
+	[WT_STATUS_UNMERGED]	  = "unmerged",
+	[WT_STATUS_LOCAL_BRANCH]  = "localBranch",
+	[WT_STATUS_REMOTE_BRANCH] = "remoteBranch",
+	[WT_STATUS_ONBRANCH]	  = "branch",
+};
+
+enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
+
+struct status_deferred_config status_deferred_config = {
+	STATUS_FORMAT_UNSPECIFIED,
+	-1, /* unspecified */
+	AHEAD_BEHIND_UNSPECIFIED,
+};
+
+int parse_status_slot(const char *slot)
+{
+	if (!strcasecmp(slot, "added"))
+		return WT_STATUS_UPDATED;
+
+	return LOOKUP_CONFIG(color_status_slots, slot);
+}
+
+int git_status_config(const char *k, const char *v,
+		      const struct config_context *ctx, void *cb)
+{
+	struct wt_status *s = cb;
+	const char *slot_name;
+
+	if (starts_with(k, "column."))
+		return git_column_config(k, v, "status", &s->colopts);
+	if (!strcmp(k, "status.submodulesummary")) {
+		int is_bool;
+		s->submodule_summary = git_config_bool_or_int(k, v, ctx->kvi,
+							      &is_bool);
+		if (is_bool && s->submodule_summary)
+			s->submodule_summary = -1;
+		return 0;
+	}
+	if (!strcmp(k, "status.short")) {
+		if (git_config_bool(k, v))
+			status_deferred_config.status_format = STATUS_FORMAT_SHORT;
+		else
+			status_deferred_config.status_format = STATUS_FORMAT_NONE;
+		return 0;
+	}
+	if (!strcmp(k, "status.noob")) {
+		if (git_config_bool(k, v))
+			status_deferred_config.status_format = STATUS_FORMAT_NOOB;
+		else
+			status_deferred_config.status_format = STATUS_FORMAT_NONE;
+		return 0;
+	}
+	if (!strcmp(k, "status.branch")) {
+		status_deferred_config.show_branch = git_config_bool(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "status.aheadbehind")) {
+		status_deferred_config.ahead_behind = git_config_bool(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "status.showstash")) {
+		s->show_stash = git_config_bool(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
+		s->use_color = git_config_colorbool(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "status.displaycommentprefix")) {
+		s->display_comment_prefix = git_config_bool(k, v);
+		return 0;
+	}
+	if (skip_prefix(k, "status.color.", &slot_name) ||
+		skip_prefix(k, "color.status.", &slot_name)) {
+		int slot = parse_status_slot(slot_name);
+		if (slot < 0)
+		       return 0;
+		if (!v)
+		       return config_error_nonbool(k);
+		return color_parse(v, s->color_palette[slot]);
+	}
+	if (!strcmp(k, "status.relativepaths")) {
+		s->relative_paths = git_config_bool(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "status.showuntrackedfiles")) {
+		if (!v)
+		       return config_error_nonbool(k);
+		else if (!strcmp(v, "no"))
+		       s->show_untracked_files = SHOW_NO_UNTRACKED_FILES;
+		else if (!strcmp(v, "normal"))
+		       s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
+		else if (!strcmp(v, "all"))
+		       s->show_untracked_files = SHOW_ALL_UNTRACKED_FILES;
+		else
+		       return error(_("Invalid untracked files mode '%s'"), v);
+		return 0;
+	}
+	if (!strcmp(k, "diff.renamelimit")) {
+		if (s->rename_limit == -1)
+		       s->rename_limit = git_config_int(k, v, ctx->kvi);
+		return 0;
+	}
+	if (!strcmp(k, "status.renamelimit")) {
+		s->rename_limit = git_config_int(k, v, ctx->kvi);
+		return 0;
+	}
+	if (!strcmp(k, "diff.renames")) {
+		if (s->detect_rename == -1)
+		       s->detect_rename = git_config_rename(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "status.renames")) {
+		s->detect_rename = git_config_rename(k, v);
+		return 0;
+	}
+	return git_diff_ui_config(k, v, ctx, NULL);
+}
+
+void finalize_deferred_config(struct wt_status *s)
+{
+	int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN &&
+				   status_format != STATUS_FORMAT_PORCELAIN_V2 &&
+				   !s->null_termination);
+
+	if (s->null_termination) {
+		if (status_format == STATUS_FORMAT_NONE ||
+		    status_format == STATUS_FORMAT_UNSPECIFIED)
+			status_format = STATUS_FORMAT_PORCELAIN;
+		else if (status_format == STATUS_FORMAT_LONG)
+			die(_("options '%s' and '%s' cannot be used together"), "--long", "-z");
+	}
+
+	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;
+
+	if (use_deferred_config && s->show_branch < 0)
+		s->show_branch = status_deferred_config.show_branch;
+	if (s->show_branch < 0)
+		s->show_branch = 0;
+
+	/*
+	 * If the user did not give a "--[no]-ahead-behind" command
+	 * line argument *AND* we will print in a human-readable format
+	 * (short, long etc.) then we inherit from the status.aheadbehind
+	 * config setting.  In all other cases (and porcelain V[12] formats
+	 * in particular), we inherit _FULL for backwards compatibility.
+	 */
+	if (use_deferred_config &&
+	    s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
+		s->ahead_behind_flags = status_deferred_config.ahead_behind;
+
+	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
+		s->ahead_behind_flags = AHEAD_BEHIND_FULL;
+}
diff --git a/noob.h b/noob.h
new file mode 100644
index 0000000000..d5bc073594
--- /dev/null
+++ b/noob.h
@@ -0,0 +1,21 @@
+#ifndef NOOB_H
+#define NOOB_H
+
+struct status_deferred_config {
+	enum wt_status_format status_format;
+	int show_branch;
+	enum ahead_behind_flags ahead_behind;
+};
+
+extern enum wt_status_format status_format;
+
+extern struct status_deferred_config status_deferred_config;
+
+int git_status_config(const char *k, const char *v,
+		      const struct config_context *ctx, void *cb);
+
+int parse_status_slot(const char *slot);
+
+void finalize_deferred_config(struct wt_status *s);
+
+#endif /* NOOB_H */
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 9a1a7edc5a..302a075714 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -4,6 +4,7 @@
 #include "hash-ll.h"
 #include "hashmap.h"
 #include "statinfo.h"
+#include "wt-status.h"
 
 /*
  * Basic data structures for the directory cache
@@ -395,6 +396,7 @@ int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_IGNORE_ERRORS	4
 #define ADD_CACHE_IGNORE_REMOVAL 8
 #define ADD_CACHE_INTENT 16
+#define ADD_CACHE_FORMAT_NOOB 32
 /*
  * These two are used to add the contents of the file at path
  * to the index, marking the working tree up-to-date by storing
@@ -404,7 +406,8 @@ int remove_file_from_index(struct index_state *, const char *path);
  * the latter will do necessary lstat(2) internally before
  * calling the former.
  */
-int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
+int add_to_index(struct index_state *, const char *path, struct stat *, int flags, struct wt_status *status);
+int add_file_to_index_with_status(struct index_state *, const char *path, int flags, struct wt_status *status);
 int add_file_to_index(struct index_state *, const char *path, int flags);
 
 int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
@@ -475,6 +478,10 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, int include_sparse,
 		       int flags);
 
+int add_files_to_cache_with_status(struct repository *repo, const char *prefix,
+		       const struct pathspec *pathspec, int include_sparse,
+		       int flags, struct wt_status *status);
+
 void overlay_tree_on_index(struct index_state *istate,
 			   const char *tree_name, const char *prefix);
 
diff --git a/read-cache.c b/read-cache.c
index 080bd39713..319415430a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -45,6 +45,8 @@
 #include "csum-file.h"
 #include "promisor-remote.h"
 #include "hook.h"
+#include "wt-status.h"
+#include "string-list.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 
@@ -664,7 +666,7 @@ void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)
 	oidcpy(&ce->oid, &oid);
 }
 
-int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
+int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags, struct wt_status *status)
 {
 	int namelen, was_same;
 	mode_t st_mode = st->st_mode;
@@ -672,6 +674,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE|CE_MATCH_RACY_IS_DIRTY;
 	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
 	int pretend = flags & ADD_CACHE_PRETEND;
+	int noob = flags & ADD_CACHE_FORMAT_NOOB;
 	int intent_only = flags & ADD_CACHE_INTENT;
 	int add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|
 			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
@@ -760,17 +763,26 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 		discard_cache_entry(ce);
 		return error(_("unable to add '%s' to index"), path);
 	}
-	if (verbose && !was_same)
+	if (verbose && !was_same && !noob)
 		printf("add '%s'\n", path);
+	if (noob && !was_same) {
+		string_list_insert(&status->added, path);
+	}
 	return 0;
 }
 
-int add_file_to_index(struct index_state *istate, const char *path, int flags)
+int add_file_to_index_with_status(struct index_state *istate, const char *path, int flags, struct wt_status *status)
 {
 	struct stat st;
 	if (lstat(path, &st))
 		die_errno(_("unable to stat '%s'"), path);
-	return add_to_index(istate, path, &st, flags);
+	return add_to_index(istate, path, &st, flags, status);
+}
+
+int add_file_to_index(struct index_state *istate, const char *path, int flags)
+{
+	struct wt_status status;
+	return add_file_to_index_with_status(istate, path, flags, &status);
 }
 
 struct cache_entry *make_empty_cache_entry(struct index_state *istate, size_t len)
@@ -3872,6 +3884,7 @@ struct update_callback_data {
 	int include_sparse;
 	int flags;
 	int add_errors;
+	struct wt_status *status;
 };
 
 static int fix_unmerged_status(struct diff_filepair *p,
@@ -3914,7 +3927,7 @@ static void update_callback(struct diff_queue_struct *q,
 			die(_("unexpected diff status %c"), p->status);
 		case DIFF_STATUS_MODIFIED:
 		case DIFF_STATUS_TYPE_CHANGED:
-			if (add_file_to_index(data->index, path, data->flags)) {
+			if (add_file_to_index_with_status(data->index, path, data->flags, data->status)) {
 				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
 					die(_("updating files failed"));
 				data->add_errors++;
@@ -3935,6 +3948,14 @@ static void update_callback(struct diff_queue_struct *q,
 int add_files_to_cache(struct repository *repo, const char *prefix,
 		       const struct pathspec *pathspec, int include_sparse,
 		       int flags)
+{
+	struct wt_status status;
+	return add_files_to_cache_with_status(repo, prefix, pathspec, include_sparse, flags, &status);
+}
+
+int add_files_to_cache_with_status(struct repository *repo, const char *prefix,
+		       const struct pathspec *pathspec, int include_sparse,
+		       int flags, struct wt_status *status)
 {
 	struct update_callback_data data;
 	struct rev_info rev;
@@ -3943,6 +3964,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 	data.index = repo->index;
 	data.include_sparse = include_sparse;
 	data.flags = flags;
+	data.status = status;
 
 	repo_init_revisions(repo, &rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
diff --git a/table.c b/table.c
index d085f2a098..527e38c07d 100644
--- a/table.c
+++ b/table.c
@@ -65,18 +65,53 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
 		strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
 }
 
-static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+static void add_arrow_to_entry(struct strbuf *buf, int add_after_entry)
+{
+	struct strbuf empty = STRBUF_INIT;
+	struct strbuf trimmed = STRBUF_INIT;
+	struct strbuf holder = STRBUF_INIT;
+	int len = strlen(buf->buf);
+
+	strbuf_addstr(&trimmed, buf->buf);
+	strbuf_trim(&trimmed);
+
+	if (!strbuf_cmp(&trimmed, &empty) && !add_after_entry) {
+		strbuf_reset(buf);
+		strbuf_addchars(buf, '-', len + 1);
+	} else if (add_after_entry) {
+		strbuf_rtrim(buf);
+		strbuf_addchars(buf, ' ', 1);
+		strbuf_addchars(buf, '-', len - strlen(buf->buf) + 1);
+	} else if (!add_after_entry) {
+		strbuf_ltrim(buf);
+		strbuf_addchars(&holder, '-', len - strlen(buf->buf) - 2);
+		strbuf_addchars(&holder, '>', 1);
+		strbuf_addchars(&holder, ' ', 1);
+		strbuf_addstr(&holder, buf->buf);
+		strbuf_reset(buf);
+		strbuf_addstr(buf, holder.buf);
+	}
+}
+
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s, int hide_pipe)
 {
 	printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
-	printf(_("|"));
+	if (hide_pipe != 1 && hide_pipe != 3)
+		printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
-	printf(_("|"));
+	if (hide_pipe != 2 && hide_pipe != 3)
+		printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
 	printf(_("|\n"));
 }
 
-void print_noob_status(struct wt_status *s, int add_advice)
+static void print_table_body_line_(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+{
+	print_table_body_line(buf1, buf2, buf3, s, 0);
+}
+
+void print_noob_status(struct wt_status *s, int advice)
 {
 	struct winsize w;
 	int cols;
@@ -84,7 +119,7 @@ void print_noob_status(struct wt_status *s, int add_advice)
 	struct strbuf table_col_entry_1 = STRBUF_INIT;
 	struct strbuf table_col_entry_2 = STRBUF_INIT;
 	struct strbuf table_col_entry_3 = STRBUF_INIT;
-	struct string_list_item *item;
+	struct string_list_item *item, *item2;
 
 	/* Get terminal width */
 	ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
@@ -104,7 +139,7 @@ void print_noob_status(struct wt_status *s, int add_advice)
 	printf(_("%s\n"), table_border.buf);
 	printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
 
-	if (add_advice) {
+	if (advice) {
 		build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
 		build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
 		build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
@@ -122,14 +157,38 @@ void print_noob_status(struct wt_status *s, int add_advice)
 
 	/* Draw table body */
 	for_each_string_list_item(item, &s->untracked) {
-		build_table_entry(&table_col_entry_1, item->string, cols);
+		struct strbuf buf_1 = STRBUF_INIT;
+		struct strbuf buf_2 = STRBUF_INIT;
+		int is_arrow = 0;
+		strbuf_addstr(&buf_1, item->string);
+		build_table_entry(&table_col_entry_1, buf_1.buf, cols);
 		build_table_entry(&table_col_entry_2, "", cols);
 		build_table_entry(&table_col_entry_3, "", cols);
-		print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+		for_each_string_list_item(item2, &s->added) {
+			strbuf_reset(&buf_2);
+			strbuf_addstr(&buf_2, item2->string);
+			if (!strbuf_cmp(&buf_1, &buf_2)) {
+				build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+				add_arrow_to_entry(&table_col_entry_1, 1);
+				add_arrow_to_entry(&table_col_entry_2, 0);
+				add_arrow_to_entry(&table_col_entry_3, 0);
+				is_arrow = 1;
+			}
+		}
+
+		if (!is_arrow)
+			print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+		else
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 3);
 	}
 
 	for_each_string_list_item(item, &s->change) {
 		struct wt_status_change_data *d = item->util;
+		struct strbuf buf_1 = STRBUF_INIT;
+		struct strbuf buf_2 = STRBUF_INIT;
+		int is_arrow = 0;
+		strbuf_addstr(&buf_1, item->string);
 		if (d->worktree_status && d->index_status) {
 			build_table_entry(&table_col_entry_1, "", cols);
 			build_table_entry(&table_col_entry_2, item->string, cols);
@@ -138,12 +197,27 @@ void print_noob_status(struct wt_status *s, int add_advice)
 			build_table_entry(&table_col_entry_1, "", cols);
 			build_table_entry(&table_col_entry_2, item->string, cols);
 			build_table_entry(&table_col_entry_3, "", cols);
+
+			for_each_string_list_item(item2, &s->added) {
+				strbuf_reset(&buf_2);
+				strbuf_addstr(&buf_2, item2->string);
+				if (!strbuf_cmp(&buf_1, &buf_2)) {
+					build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+					add_arrow_to_entry(&table_col_entry_2, 1);
+					add_arrow_to_entry(&table_col_entry_3, 0);
+					is_arrow = 1;
+				}
+			}
 		} else if (d->index_status) {
 			build_table_entry(&table_col_entry_1, "", cols);
 			build_table_entry(&table_col_entry_2, "", cols);
 			build_table_entry(&table_col_entry_3, item->string, cols);
 		}
-		print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+		if (!is_arrow)
+			print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+		else
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 2);
 	}
 	
 	if (!s->untracked.nr && !s->change.nr) {
diff --git a/wt-status.c b/wt-status.c
index b5899dcc98..969f79f441 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -153,6 +153,7 @@ void wt_status_prepare(struct repository *r, struct wt_status *s)
 	s->change.strdup_strings = 1;
 	s->untracked.strdup_strings = 1;
 	s->ignored.strdup_strings = 1;
+	s->added.strdup_strings = 1;
 	s->show_branch = -1;  /* unspecified */
 	s->show_stash = 0;
 	s->ahead_behind_flags = AHEAD_BEHIND_UNSPECIFIED;
diff --git a/wt-status.h b/wt-status.h
index 3f08f0d72b..64551f3a75 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -142,6 +142,7 @@ struct wt_status {
 	struct string_list change;
 	struct string_list untracked;
 	struct string_list ignored;
+	struct string_list added;
 	uint32_t untracked_in_ms;
 };
 
-- 
2.42.0.404.g2bcc23f3db

[RFC PATCH v2 4/6] add: set unique color for noob mode arrows

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 table.c     | 64 +++++++++++++++++++++++++++++++----------------------
 wt-status.c |  1 +
 wt-status.h |  1 +
 3 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/table.c b/table.c
index 527e38c07d..d29b311440 100644
--- a/table.c
+++ b/table.c
@@ -5,6 +5,7 @@
 #include "wt-status.h"
 #include "config.h"
 #include "string-list.h"
+#include "color.h"
 #include "sys/ioctl.h"
 
 static const char *color(int slot, struct wt_status *s)
@@ -26,7 +27,7 @@ static void build_table_border(struct strbuf *buf, int cols)
 static void build_table_entry(struct strbuf *buf, char *entry, int cols)
 {
 	int len = strlen(entry);
-	size_t col_width = (cols / 3) - 5; /* subtract for padding */
+	size_t col_width = (cols / 3) - 9; /* subtract for padding */
 
 	strbuf_reset(buf);
 
@@ -65,52 +66,51 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
 		strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
 }
 
-static void add_arrow_to_entry(struct strbuf *buf, int add_after_entry)
+static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry)
 {
 	struct strbuf empty = STRBUF_INIT;
 	struct strbuf trimmed = STRBUF_INIT;
-	struct strbuf holder = STRBUF_INIT;
 	int len = strlen(buf->buf);
 
+	strbuf_reset(arrow);
 	strbuf_addstr(&trimmed, buf->buf);
 	strbuf_trim(&trimmed);
 
 	if (!strbuf_cmp(&trimmed, &empty) && !add_after_entry) {
 		strbuf_reset(buf);
-		strbuf_addchars(buf, '-', len + 1);
+		strbuf_addchars(arrow, '-', len + 1);
 	} else if (add_after_entry) {
 		strbuf_rtrim(buf);
-		strbuf_addchars(buf, ' ', 1);
-		strbuf_addchars(buf, '-', len - strlen(buf->buf) + 1);
+		strbuf_addchars(arrow, ' ', 1);
+		strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
 	} else if (!add_after_entry) {
 		strbuf_ltrim(buf);
-		strbuf_addchars(&holder, '-', len - strlen(buf->buf) - 2);
-		strbuf_addchars(&holder, '>', 1);
-		strbuf_addchars(&holder, ' ', 1);
-		strbuf_addstr(&holder, buf->buf);
-		strbuf_reset(buf);
-		strbuf_addstr(buf, holder.buf);
+		strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
+		strbuf_addchars(arrow, '>', 1);
+		strbuf_addchars(arrow, ' ', 1);
 	}
 }
 
-static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s, int hide_pipe)
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct strbuf *arrow1, struct strbuf *arrow2, struct strbuf *arrow3, struct wt_status *s, int hide_pipe)
 {
 	printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
+	if (strlen(arrow1->buf) > 0)
+		color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow1->buf);
 	if (hide_pipe != 1 && hide_pipe != 3)
 		printf(_("|"));
 	color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
+	if (strlen(arrow2->buf) > 0)
+		color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow2->buf);
 	if (hide_pipe != 2 && hide_pipe != 3)
 		printf(_("|"));
+	if (strlen(arrow3->buf) > 0) {
+		color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow3->buf);
+	}
 	color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
 	printf(_("|\n"));
 }
 
-static void print_table_body_line_(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
-{
-	print_table_body_line(buf1, buf2, buf3, s, 0);
-}
-
 void print_noob_status(struct wt_status *s, int advice)
 {
 	struct winsize w;
@@ -119,6 +119,9 @@ void print_noob_status(struct wt_status *s, int advice)
 	struct strbuf table_col_entry_1 = STRBUF_INIT;
 	struct strbuf table_col_entry_2 = STRBUF_INIT;
 	struct strbuf table_col_entry_3 = STRBUF_INIT;
+	struct strbuf arrow_1 = STRBUF_INIT;
+	struct strbuf arrow_2 = STRBUF_INIT;
+	struct strbuf arrow_3 = STRBUF_INIT;
 	struct string_list_item *item, *item2;
 
 	/* Get terminal width */
@@ -170,17 +173,21 @@ void print_noob_status(struct wt_status *s, int advice)
 			strbuf_addstr(&buf_2, item2->string);
 			if (!strbuf_cmp(&buf_1, &buf_2)) {
 				build_table_entry(&table_col_entry_3, buf_1.buf, cols);
-				add_arrow_to_entry(&table_col_entry_1, 1);
-				add_arrow_to_entry(&table_col_entry_2, 0);
-				add_arrow_to_entry(&table_col_entry_3, 0);
+				build_arrow(&table_col_entry_1, &arrow_1, 1);
+				build_arrow(&table_col_entry_2, &arrow_2, 0);
+				build_arrow(&table_col_entry_3, &arrow_3, 0);
 				is_arrow = 1;
 			}
 		}
 
 		if (!is_arrow)
-			print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
 		else
-			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 3);
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 3);
+
+		strbuf_reset(&arrow_1);
+		strbuf_reset(&arrow_2);
+		strbuf_reset(&arrow_3);
 	}
 
 	for_each_string_list_item(item, &s->change) {
@@ -203,8 +210,8 @@ void print_noob_status(struct wt_status *s, int advice)
 				strbuf_addstr(&buf_2, item2->string);
 				if (!strbuf_cmp(&buf_1, &buf_2)) {
 					build_table_entry(&table_col_entry_3, buf_1.buf, cols);
-					add_arrow_to_entry(&table_col_entry_2, 1);
-					add_arrow_to_entry(&table_col_entry_3, 0);
+					build_arrow(&table_col_entry_2, &arrow_2, 1);
+					build_arrow(&table_col_entry_3, &arrow_3, 0);
 					is_arrow = 1;
 				}
 			}
@@ -215,9 +222,12 @@ void print_noob_status(struct wt_status *s, int advice)
 		}
 
 		if (!is_arrow)
-			print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
 		else
-			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 2);
+			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 2);
+		strbuf_reset(&arrow_1);
+		strbuf_reset(&arrow_2);
+		strbuf_reset(&arrow_3);
 	}
 	
 	if (!s->untracked.nr && !s->change.nr) {
diff --git a/wt-status.c b/wt-status.c
index 969f79f441..1332d07dba 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -49,6 +49,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_GREEN,  /* WT_STATUS_LOCAL_BRANCH */
 	GIT_COLOR_RED,    /* WT_STATUS_REMOTE_BRANCH */
 	GIT_COLOR_NIL,    /* WT_STATUS_ONBRANCH */
+	GIT_COLOR_CYAN,   /* WT_STATUS_ARROW */
 };
 
 static const char *color(int slot, struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 64551f3a75..7b883fd476 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -19,6 +19,7 @@ enum color_wt_status {
 	WT_STATUS_LOCAL_BRANCH,
 	WT_STATUS_REMOTE_BRANCH,
 	WT_STATUS_ONBRANCH,
+	WT_STATUS_ARROW,
 	WT_STATUS_MAXSLOT
 };
 
-- 
2.42.0.404.g2bcc23f3db

[RFC PATCH v2 5/6] restore: implement noob mode

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 builtin/checkout.c | 46 ++++++++++++++++++++++++++++-------
 read-cache-ll.h    |  1 +
 read-cache.c       |  9 ++++++-
 table.c            | 60 +++++++++++++++++++++++++++++++++++++++-------
 wt-status.h        |  1 +
 5 files changed, 100 insertions(+), 17 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index f02434bc15..afc414b0b1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -41,6 +41,7 @@
 #include "entry.h"
 #include "parallel-checkout.h"
 #include "add-interactive.h"
+#include "noob.h"
 
 static const char * const checkout_usage[] = {
 	N_("git checkout [<options>] <branch>"),
@@ -456,7 +457,8 @@ static int checkout_worktree(const struct checkout_opts *opts,
 }
 
 static int checkout_paths(const struct checkout_opts *opts,
-			  const struct branch_info *new_branch_info)
+			  const struct branch_info *new_branch_info,
+			  struct wt_status *status)
 {
 	int pos;
 	static char *ps_matched;
@@ -598,8 +600,10 @@ static int checkout_paths(const struct checkout_opts *opts,
 	for (pos = 0; pos < the_index.cache_nr; pos++) {
 		const struct cache_entry *ce = the_index.cache[pos];
 		if (ce->ce_flags & CE_MATCHED) {
-			if (!ce_stage(ce))
+			if (!ce_stage(ce)) {
+				string_list_insert(&status->restored, ce->name);
 				continue;
+			}
 			if (opts->ignore_unmerged) {
 				if (!opts->quiet)
 					warning(_("path '%s' is unmerged"), ce->name);
@@ -621,7 +625,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 	if (opts->checkout_worktree)
 		errs |= checkout_worktree(opts, new_branch_info);
 	else
-		remove_marked_cache_entries(&the_index, 1);
+		remove_marked_cache_entries_with_status(&the_index, 1, status);
 
 	/*
 	 * Allow updating the index when checking out from the index.
@@ -1668,7 +1672,8 @@ static char cb_option = 'b';
 static int checkout_main(int argc, const char **argv, const char *prefix,
 			 struct checkout_opts *opts, struct option *options,
 			 const char * const usagestr[],
-			 struct branch_info *new_branch_info)
+			 struct branch_info *new_branch_info,
+			 struct wt_status *status)
 {
 	int parseopt_flags = 0;
 
@@ -1865,7 +1870,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 	}
 
 	if (opts->patch_mode || opts->pathspec.nr)
-		return checkout_paths(opts, new_branch_info);
+		return checkout_paths(opts, new_branch_info, status);
 	else
 		return checkout_branch(opts, new_branch_info);
 }
@@ -1887,6 +1892,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	};
 	int ret;
 	struct branch_info new_branch_info = { 0 };
+	struct wt_status status;
 
 	memset(&opts, 0, sizeof(opts));
 	opts.dwim_new_local_branch = 1;
@@ -1917,7 +1923,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	options = add_checkout_path_options(&opts, options);
 
 	ret = checkout_main(argc, argv, prefix, &opts,
-			    options, checkout_usage, &new_branch_info);
+			    options, checkout_usage,
+			    &new_branch_info, &status);
 	branch_info_release(&new_branch_info);
 	clear_pathspec(&opts.pathspec);
 	free(opts.pathspec_from_file);
@@ -1942,6 +1949,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
 	};
 	int ret;
 	struct branch_info new_branch_info = { 0 };
+	struct wt_status status;
 
 	memset(&opts, 0, sizeof(opts));
 	opts.dwim_new_local_branch = 1;
@@ -1961,7 +1969,8 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
 	cb_option = 'c';
 
 	ret = checkout_main(argc, argv, prefix, &opts,
-			    options, switch_branch_usage, &new_branch_info);
+			    options, switch_branch_usage,
+			    &new_branch_info, &status);
 	branch_info_release(&new_branch_info);
 	FREE_AND_NULL(options);
 	return ret;
@@ -1985,6 +1994,13 @@ int cmd_restore(int argc, const char **argv, const char *prefix)
 	};
 	int ret;
 	struct branch_info new_branch_info = { 0 };
+	struct wt_status status;
+	unsigned int progress_flag = 0;
+
+	wt_status_prepare(the_repository, &status);
+	git_config(git_status_config, &status);
+	finalize_deferred_config(&status);
+	status.status_format = status_format;
 
 	memset(&opts, 0, sizeof(opts));
 	opts.accept_ref = 0;
@@ -2000,7 +2016,21 @@ int cmd_restore(int argc, const char **argv, const char *prefix)
 	options = add_checkout_path_options(&opts, options);
 
 	ret = checkout_main(argc, argv, prefix, &opts,
-			    options, restore_usage, &new_branch_info);
+			    options, restore_usage,
+			    &new_branch_info, &status);
+
+	if (status.status_format == STATUS_FORMAT_NOOB) {
+		/* Read index and populate status */
+		repo_read_index(the_repository);
+		refresh_index(&the_index,
+			      REFRESH_QUIET|REFRESH_UNMERGED|progress_flag,
+			      &status.pathspec, NULL, NULL);
+		status.show_branch = 0;
+		wt_status_collect(&status);
+		wt_status_print(&status);
+		wt_status_collect_free_buffers(&status);
+	}
+
 	branch_info_release(&new_branch_info);
 	FREE_AND_NULL(options);
 	return ret;
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 302a075714..8bdc157196 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -389,6 +389,7 @@ void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
 /* Remove entry, return true if there are more entries to go. */
 int remove_index_entry_at(struct index_state *, int pos);
 
+void remove_marked_cache_entries_with_status(struct index_state *istate, int invalidate, struct wt_status *status);
 void remove_marked_cache_entries(struct index_state *istate, int invalidate);
 int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_VERBOSE 1
diff --git a/read-cache.c b/read-cache.c
index 319415430a..1c1a3290c0 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -558,7 +558,7 @@ int remove_index_entry_at(struct index_state *istate, int pos)
  * CE_REMOVE is set in ce_flags.  This is much more effective than
  * calling remove_index_entry_at() for each entry to be removed.
  */
-void remove_marked_cache_entries(struct index_state *istate, int invalidate)
+void remove_marked_cache_entries_with_status(struct index_state *istate, int invalidate, struct wt_status *status)
 {
 	struct cache_entry **ce_array = istate->cache;
 	unsigned int i, j;
@@ -570,6 +570,7 @@ void remove_marked_cache_entries(struct index_state *istate, int invalidate)
 							   ce_array[i]->name);
 				untracked_cache_remove_from_index(istate,
 								  ce_array[i]->name);
+				string_list_insert(&status->restored, ce_array[i]->name);
 			}
 			remove_name_hash(istate, ce_array[i]);
 			save_or_free_index_entry(istate, ce_array[i]);
@@ -583,6 +584,12 @@ void remove_marked_cache_entries(struct index_state *istate, int invalidate)
 	istate->cache_nr = j;
 }
 
+void remove_marked_cache_entries(struct index_state *istate, int invalidate)
+{
+	struct wt_status status;
+	remove_marked_cache_entries_with_status(istate, invalidate, &status);
+}
+
 int remove_file_from_index(struct index_state *istate, const char *path)
 {
 	int pos = index_name_pos(istate, path, strlen(path));
diff --git a/table.c b/table.c
index d29b311440..3602def17a 100644
--- a/table.c
+++ b/table.c
@@ -66,7 +66,7 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
 		strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
 }
 
-static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry)
+static void build_arrow_(struct strbuf *buf, struct strbuf* arrow, int add_after_entry, int reversed)
 {
 	struct strbuf empty = STRBUF_INIT;
 	struct strbuf trimmed = STRBUF_INIT;
@@ -80,17 +80,38 @@ static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_
 		strbuf_reset(buf);
 		strbuf_addchars(arrow, '-', len + 1);
 	} else if (add_after_entry) {
-		strbuf_rtrim(buf);
-		strbuf_addchars(arrow, ' ', 1);
-		strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
+		if (!reversed) {
+			strbuf_rtrim(buf);
+			strbuf_addchars(arrow, ' ', 1);
+			strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
+		} else {
+			strbuf_rtrim(buf);
+			strbuf_addchars(arrow, ' ', 1);
+			strbuf_addchars(arrow, '<', 1);
+			strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
+		}
 	} else if (!add_after_entry) {
-		strbuf_ltrim(buf);
-		strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
-		strbuf_addchars(arrow, '>', 1);
-		strbuf_addchars(arrow, ' ', 1);
+		if (!reversed) {
+			strbuf_ltrim(buf);
+			strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
+			strbuf_addchars(arrow, '>', 1);
+			strbuf_addchars(arrow, ' ', 1);
+		} else {
+			strbuf_ltrim(buf);
+			strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
+			strbuf_addchars(arrow, ' ', 1);
+		}
 	}
 }
 
+static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry) {
+	build_arrow_(buf, arrow, add_after_entry, 0);
+}
+
+static void build_reversed_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry) {
+	build_arrow_(buf, arrow, add_after_entry, 1);
+}
+
 static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct strbuf *arrow1, struct strbuf *arrow2, struct strbuf *arrow3, struct wt_status *s, int hide_pipe)
 {
 	printf(_("|"));
@@ -180,6 +201,18 @@ void print_noob_status(struct wt_status *s, int advice)
 			}
 		}
 
+		for_each_string_list_item(item2, &s->restored) {
+			strbuf_reset(&buf_2);
+			strbuf_addstr(&buf_2, item2->string);
+			if (!strbuf_cmp(&buf_1, &buf_2)) {
+				build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+				build_reversed_arrow(&table_col_entry_1, &arrow_1, 1);
+				build_reversed_arrow(&table_col_entry_2, &arrow_2, 0);
+				build_reversed_arrow(&table_col_entry_3, &arrow_3, 0);
+				is_arrow = 1;
+			}
+		}
+
 		if (!is_arrow)
 			print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
 		else
@@ -215,6 +248,17 @@ void print_noob_status(struct wt_status *s, int advice)
 					is_arrow = 1;
 				}
 			}
+
+			for_each_string_list_item(item2, &s->restored) {
+				strbuf_reset(&buf_2);
+				strbuf_addstr(&buf_2, item2->string);
+				if (!strbuf_cmp(&buf_1, &buf_2)) {
+					build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+					build_reversed_arrow(&table_col_entry_2, &arrow_2, 1);
+					build_reversed_arrow(&table_col_entry_3, &arrow_3, 0);
+					is_arrow = 1;
+				}
+			}
 		} else if (d->index_status) {
 			build_table_entry(&table_col_entry_1, "", cols);
 			build_table_entry(&table_col_entry_2, "", cols);
diff --git a/wt-status.h b/wt-status.h
index 7b883fd476..c6bce8f74a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -144,6 +144,7 @@ struct wt_status {
 	struct string_list untracked;
 	struct string_list ignored;
 	struct string_list added;
+	struct string_list restored;
 	uint32_t untracked_in_ms;
 };
 
-- 
2.42.0.404.g2bcc23f3db

[RFC PATCH v2 6/6] status: add advice status hints as table footer

[Jacob Stopak]

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 builtin/commit.c |  1 +
 table.c          | 42 +++++++++++++++++++++++++++---------------
 table.h          |  2 +-
 wt-status.c      |  3 ++-
 wt-status.h      |  2 ++
 5 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 3f816c117d..b97943e642 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1434,6 +1434,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	s.ignore_submodule_arg = ignore_submodule_arg;
 	s.status_format = status_format;
 	s.verbose = verbose;
+	s.is_cmd_status = 1;
 	if (no_renames != -1)
 		s.detect_rename = !no_renames;
 	if ((intptr_t)rename_score_arg != -1) {
diff --git a/table.c b/table.c
index 3602def17a..36719e3d09 100644
--- a/table.c
+++ b/table.c
@@ -6,6 +6,7 @@
 #include "config.h"
 #include "string-list.h"
 #include "color.h"
+#include "advice.h"
 #include "sys/ioctl.h"
 
 static const char *color(int slot, struct wt_status *s)
@@ -132,7 +133,18 @@ static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, stru
 	printf(_("|\n"));
 }
 
-void print_noob_status(struct wt_status *s, int advice)
+static void print_table_hint_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+{
+	printf(_("|"));
+	color_fprintf(s->fp, color(WT_STATUS_HINT, s), "%s", buf1->buf);
+	printf(_("|"));
+	color_fprintf(s->fp, color(WT_STATUS_HINT, s), "%s", buf2->buf);
+	printf(_("|"));
+	color_fprintf(s->fp, color(WT_STATUS_HINT, s), "%s", buf3->buf);
+	printf(_("|\n"));
+}
+
+void print_noob_status(struct wt_status *s)
 {
 	struct winsize w;
 	int cols;
@@ -163,20 +175,6 @@ void print_noob_status(struct wt_status *s, int advice)
 	printf(_("%s\n"), table_border.buf);
 	printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
 
-	if (advice) {
-		build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
-		build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
-		build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
-
-		printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
-
-		build_table_entry(&table_col_entry_1, "", cols);
-		build_table_entry(&table_col_entry_2, "(discard: git restore --staged <file>)", cols);
-		build_table_entry(&table_col_entry_3, "", cols);
-
-		printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
-	}
-
 	printf(_("%s\n"), table_border.buf);
 
 	/* Draw table body */
@@ -282,6 +280,20 @@ void print_noob_status(struct wt_status *s, int advice)
 	}
 
 	printf(_("%s\n"), table_border.buf);
+
+	if (s->is_cmd_status && advice_enabled(ADVICE_STATUS_HINTS)) {
+		build_table_entry(&table_col_entry_1, "stage: git add ...", cols);
+		build_table_entry(&table_col_entry_2, "stage: git add ...", cols);
+		build_table_entry(&table_col_entry_3, "unstage: git restore --staged ...", cols);
+		print_table_hint_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+		build_table_entry(&table_col_entry_1, "", cols);
+		build_table_entry(&table_col_entry_2, "discard: git restore --staged ...", cols);
+		build_table_entry(&table_col_entry_3, "", cols);
+		print_table_hint_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+		printf(_("%s\n"), table_border.buf);
+	}
+
 	strbuf_release(&table_border);
 	strbuf_release(&table_col_entry_1);
 	strbuf_release(&table_col_entry_2);
diff --git a/table.h b/table.h
index 5dff7162a4..c9e8c386de 100644
--- a/table.h
+++ b/table.h
@@ -1,6 +1,6 @@
 #ifndef TABLE_H
 #define TABLE_H
 
-void print_noob_status(struct wt_status *s, int i);
+void print_noob_status(struct wt_status *s);
 
 #endif /* TABLE_H */
diff --git a/wt-status.c b/wt-status.c
index 1332d07dba..288817dcf7 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -50,6 +50,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RED,    /* WT_STATUS_REMOTE_BRANCH */
 	GIT_COLOR_NIL,    /* WT_STATUS_ONBRANCH */
 	GIT_COLOR_CYAN,   /* WT_STATUS_ARROW */
+	GIT_COLOR_YELLOW, /* WT_STATUS_HINT */
 };
 
 static const char *color(int slot, struct wt_status *s)
@@ -2151,7 +2152,7 @@ static void wt_noobstatus_print(struct wt_status *s)
 		wt_longstatus_print_tracking(s);
 	}
 
-	print_noob_status(s, 0);
+	print_noob_status(s);
 }
 
 static void wt_porcelain_print(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index c6bce8f74a..0a14b4b064 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -20,6 +20,7 @@ enum color_wt_status {
 	WT_STATUS_REMOTE_BRANCH,
 	WT_STATUS_ONBRANCH,
 	WT_STATUS_ARROW,
+	WT_STATUS_HINT,
 	WT_STATUS_MAXSLOT
 };
 
@@ -146,6 +147,7 @@ struct wt_status {
 	struct string_list added;
 	struct string_list restored;
 	uint32_t untracked_in_ms;
+	int is_cmd_status;
 };
 
 size_t wt_status_locate_end(const char *s, size_t len);
-- 
2.42.0.404.g2bcc23f3db

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Dragan Simic]

On 2023-10-27 00:46, Jacob Stopak wrote:
> Take into account reviewer feedback by doing several things 
> differently:
> 
>   * Rename this feature (for now) as "noob format mode" (or just "noob
>     mode") instead of the original "--table" verbiage. As pointed out,
>     this no longer ties the name of the setting to it's proposed
>     implementation detail as a table. Noob mode is not necessarily the
>     right name, just a placeholder for now. Unless people like it :D
> 
>   * Instead of manually having to invoke the -t, --table every time 
> this
>     format is to be used, set the config option "status.noob" to true.
>     Although this is logically tied to the status command, there are 
> many
>     commands that produce status output, (and this series adds more), 
> so
>     assume that if the user wants to see the status this way, that it
>     should be enabled whenever the status info is displayed.

How would "status.noob" relate to and coexist with possible future 
configuration options named "<command>.verbose", which would be somewhat 
similar to the currently existing "commit.verbose" option?  IOW, perhaps 
it would be better to have per-command options "<command>.verbose = 
noob" or, even better, "<command>.verbose = extended", to make it all 
more future-proof and more granular.

>   * When running "git add" and "git restore" while noob mode is 
> enabled,
>     perform the add/restore function as usual, but display the table
>     formatted output with arrows showing how file changes moved around.
>     Displaying the output in this understandable format after each
>     command execution allows the noob to immediately see what they did.
>     Although this series only implements for status, add, and restore,
>     this output format would make sense in other commands like rm, mv,
>     commit, clean, and stash.
> 
>   * Works consistently with commands that already have a --dry-run
>     (-n) option. The dry run shows the exact same output, but
>     doesn't actually do the thing.
> 
>   * If `advice.statusHints` is true, add a table footer with status 
> hints.
>     Shorten these hints so that they are still clear but better fit 
> into a
>     table. Make the hint text yellow to distinguish them. The hints 
> only
>     appear when explicitly running "git status", which helps the user
>     answer the question "what can I do next?". Hints are omitted in
>     "impact" commands like add and restore. Having hints here distracts
>     from the file change moves being showed in the table by arrows.
> 
> TODO:
> 
>   * "git status" outputs myriad other information depending on the 
> state
>     of the repo, like branch info, merge conflicts, rebase info, 
> bisect,
>     etc. Need to think about how to convey that info with the new 
> setting.
> 
>   * Some commands (like stash) might need more than 3 table columns to
>     display everything clearly.
> 
>   * For destructive commands, think about adding a prompt describing 
> the
>     effect, so the user can confirm before the action is taken.
> 
>   * Fix horrible things in the patch series code.
> 
>   * Probably other things.
> 
> Play around with it! It's fun!
> 
> Jacob Stopak (6):
>   status: add noob format from status.noob config
>   status: handle long paths in noob format
>   add: implement noob mode
>   add: set unique color for noob mode arrows
>   restore: implement noob mode
>   status: add advice status hints as table footer
> 
>  Makefile           |   2 +
>  builtin/add.c      |  47 +++++--
>  builtin/checkout.c |  46 +++++--
>  builtin/commit.c   | 157 +----------------------
>  commit.c           |   2 +
>  noob.c             | 198 +++++++++++++++++++++++++++++
>  noob.h             |  21 ++++
>  read-cache-ll.h    |  10 +-
>  read-cache.c       |  41 +++++-
>  table.c            | 301 +++++++++++++++++++++++++++++++++++++++++++++
>  table.h            |   6 +
>  wt-status.c        |  75 +++++++----
>  wt-status.h        |   6 +
>  13 files changed, 708 insertions(+), 204 deletions(-)
>  create mode 100644 noob.c
>  create mode 100644 noob.h
>  create mode 100644 table.c
>  create mode 100644 table.h

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Jacob Stopak]

On Fri, Oct 27, 2023 at 03:32:40PM +0200, Dragan Simic wrote:
> On 2023-10-27 00:46, Jacob Stopak wrote:
> > Take into account reviewer feedback by doing several things differently:
> > 
> >   * Rename this feature (for now) as "noob format mode" (or just "noob
> >     mode") instead of the original "--table" verbiage. As pointed out,
> >     this no longer ties the name of the setting to it's proposed
> >     implementation detail as a table. Noob mode is not necessarily the
> >     right name, just a placeholder for now. Unless people like it :D
> > 
> >   * Instead of manually having to invoke the -t, --table every time this
> >     format is to be used, set the config option "status.noob" to true.
> >     Although this is logically tied to the status command, there are
> > many
> >     commands that produce status output, (and this series adds more), so
> >     assume that if the user wants to see the status this way, that it
> >     should be enabled whenever the status info is displayed.
> 
> How would "status.noob" relate to and coexist with possible future
> configuration options named "<command>.verbose", which would be somewhat
> similar to the currently existing "commit.verbose" option?  IOW, perhaps it
> would be better to have per-command options "<command>.verbose = noob" or,
> even better, "<command>.verbose = extended", to make it all more
> future-proof and more granular.

Hmm, do there currently exist other <command>.verbose config settings
besides for commit? From what I can tell from "git help config", the
commit.verbose setting is the only one I see, and it just adds the diff
info into the editor if the user runs git commit without the -m flag, but
otherwise there seems to be no extra verbosity outputted.

I noticed that git add and git mv have "verbose" (-v, --verbose) cli flags
which just output the name of the file being added or renamed, and that
certain other commands like git branch has a verbose output which includes
the branch head commit hash and message in the output, so I guess this one
is actually kindof verbose in that it outputs more than the non-empty
default output.

So it seems like currently "verbose" is used for various things among the
command set, sometimes meaning "add something into the template if one is
used" or "add some tiny output to a command that has no default output"
(which still seems more "--shy" than "--verbose" :P) or "add some
additional output to a command that already has some sparse output".

Another thing is that commands like status have multiple flags that can be
used to specify the output format, such as --short, --long, --porcelain,
etc, but only --short seems to be configurable as a git config setting.
Is there a reason (besides backward compatibility I guess) that these
aren't rolled into a single thing like --format=<type>? This seems like
it would be the easiest way to future proof for new formats like
--format=verbose, --format=noob, --format=extended, etc.

From a noob's perspective though, does adding a config setting for each
command really make sense? I'm kindof envisioning this setting now as a
"mode" that is either enabled for all commands it affects or for none.
And it's highly unlikely a newish user would individually discover which
commands this "extended" format is available for, and run "git config
<command>.verbose = extended" for every one. I mean we could do that
in case there are folks who only want it for specific commands, but to
fulfill it's purpose I think there should definetely be some general way
to enable the setting for all commands that have it.

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Dragan Simic]

On 2023-10-27 19:13, Jacob Stopak wrote:
> On Fri, Oct 27, 2023 at 03:32:40PM +0200, Dragan Simic wrote:
>> On 2023-10-27 00:46, Jacob Stopak wrote:
>> > Take into account reviewer feedback by doing several things differently:
>> >
>> >   * Rename this feature (for now) as "noob format mode" (or just "noob
>> >     mode") instead of the original "--table" verbiage. As pointed out,
>> >     this no longer ties the name of the setting to it's proposed
>> >     implementation detail as a table. Noob mode is not necessarily the
>> >     right name, just a placeholder for now. Unless people like it :D
>> >
>> >   * Instead of manually having to invoke the -t, --table every time this
>> >     format is to be used, set the config option "status.noob" to true.
>> >     Although this is logically tied to the status command, there are
>> > many
>> >     commands that produce status output, (and this series adds more), so
>> >     assume that if the user wants to see the status this way, that it
>> >     should be enabled whenever the status info is displayed.
>> 
>> How would "status.noob" relate to and coexist with possible future
>> configuration options named "<command>.verbose", which would be 
>> somewhat
>> similar to the currently existing "commit.verbose" option?  IOW, 
>> perhaps it
>> would be better to have per-command options "<command>.verbose = noob" 
>> or,
>> even better, "<command>.verbose = extended", to make it all more
>> future-proof and more granular.
> 
> Hmm, do there currently exist other <command>.verbose config settings
> besides for commit? From what I can tell from "git help config", the
> commit.verbose setting is the only one I see, and it just adds the diff
> info into the editor if the user runs git commit without the -m flag, 
> but
> otherwise there seems to be no extra verbosity outputted.

They currently don't exist, but that's something I've planned to 
implement, e.g. to "add.verbose" as a new configuration option.  It 
should be usable, while not being messy or intrusive as a new feature.

> I noticed that git add and git mv have "verbose" (-v, --verbose) cli 
> flags
> which just output the name of the file being added or renamed, and that
> certain other commands like git branch has a verbose output which 
> includes
> the branch head commit hash and message in the output, so I guess this 
> one
> is actually kindof verbose in that it outputs more than the non-empty
> default output.

Yes, those are the basic per-command verbosity modes or levels, as I 
call them.  The way I see it, your patches would add new, extended 
per-command verbosity levels.

> So it seems like currently "verbose" is used for various things among 
> the
> command set, sometimes meaning "add something into the template if one 
> is
> used" or "add some tiny output to a command that has no default output"
> (which still seems more "--shy" than "--verbose" :P) or "add some
> additional output to a command that already has some sparse output".

Yes, that's the basic verbosity, as I named it above.

> Another thing is that commands like status have multiple flags that can 
> be
> used to specify the output format, such as --short, --long, 
> --porcelain,
> etc, but only --short seems to be configurable as a git config setting.
> Is there a reason (besides backward compatibility I guess) that these
> aren't rolled into a single thing like --format=<type>? This seems like
> it would be the easiest way to future proof for new formats like
> --format=verbose, --format=noob, --format=extended, etc.

That's a good question, but I'd need to go through the commit history to 
be able to provide some kind of an explanation.  It could also be all 
packed into "status.verbose" as a new configuration option.

> From a noob's perspective though, does adding a config setting for each
> command really make sense? I'm kindof envisioning this setting now as a
> "mode" that is either enabled for all commands it affects or for none.
> And it's highly unlikely a newish user would individually discover 
> which
> commands this "extended" format is available for, and run "git config
> <command>.verbose = extended" for every one. I mean we could do that
> in case there are folks who only want it for specific commands, but to
> fulfill it's purpose I think there should definetely be some general 
> way
> to enable the setting for all commands that have it.

Quite frankly, we shouldn't expect that all users are noobs, and as a 
result dumb everything down just to make them as comfortable as 
possible.  On the other hand, perhaps not everyone would like to have 
extended verbosity enabled for all commands, just as not everyone uses 
"-v" for all commands.

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Jacob Stopak]

> They currently don't exist, but that's something I've planned to implement,
> e.g. to "add.verbose" as a new configuration option.  It should be usable,
> while not being messy or intrusive as a new feature.

"git add" already has the -v, --verbose flag available for the command
itself like:

$ git add -v foo.txt
add 'foo.txt'

But like you said the config option add.verbose doesn't seem to exist yet.

So I assume an "add.verbose" config option would just always print that
without having to specify the -v, --verbose flag when running the command?

Basically what I'm asking is if commands that already have a --verbose flag
would just get a config setting that does the existing thing by default?

> Yes, those are the basic per-command verbosity modes or levels, as I call
> them.  The way I see it, your patches would add new, extended per-command
> verbosity levels.

Ok, I see.

> > So it seems like currently "verbose" is used for various things among
> > the command set...
> Yes, that's the basic verbosity, as I named it above.

Would it make sense to try to define a more consistent type of output or
format style for "verbosity" across different commands? As it stands it
seems each command treats verbosity in its own way which makes it hard to
interpret exactly what it will do each time...

> > Another thing is that commands like status have multiple flags that can
> > be
> > used to specify the output format, such as --short, --long, --porcelain,
> > etc, but only --short seems to be configurable as a git config setting.
> > Is there a reason (besides backward compatibility I guess) that these
> > aren't rolled into a single thing like --format=<type>? This seems like
> > it would be the easiest way to future proof for new formats like
> > --format=verbose, --format=noob, --format=extended, etc.
> 
> That's a good question, but I'd need to go through the commit history to be
> able to provide some kind of an explanation.  It could also be all packed
> into "status.verbose" as a new configuration option.

Ok so it sounds like you prefer to use "verbose" as the setting key?
I guess at this point that might make more sense since commit.verbose
already exists, and existing options could be packed into it like you
said instead of just true or false.

And then my thing here would just be called "command.verbose = extended"?

> > From a noob's perspective though, does adding a config setting for each
> > command really make sense? I'm kindof envisioning this setting now as a
> > "mode" that is either enabled for all commands it affects or for none.
> > And it's highly unlikely a newish user would individually discover which
> > commands this "extended" format is available for, and run "git config
> > <command>.verbose = extended" for every one. I mean we could do that
> > in case there are folks who only want it for specific commands, but to
> > fulfill it's purpose I think there should definetely be some general way
> > to enable the setting for all commands that have it.
> 
> Quite frankly, we shouldn't expect that all users are noobs, and as a result
> dumb everything down just to make them as comfortable as possible.  On the
> other hand, perhaps not everyone would like to have extended verbosity
> enabled for all commands, just as not everyone uses "-v" for all commands.

I agree with this, and I think it's important to cater to both newbies and
experienced users alike. That's why I said I never dreamed of making this
new format the default.

And it's true that some users might only want the extended (or any format)
for specific commands. I think a happy medium then is to have the command-
specific settings like you mention, plus one toplevel option that enables a
specific type of output format among all commands (and overrides the
command-specific settings), so that the user can choose which they prefer.

Any thoughts on what the section in the config for a more general setting
like this might be named? If "status.verbose = extended" would already be
taken specifically for the status command, what terminology could we use
to mean something like "global.verbose = extended" or "global.extended =
true"? Although the former seems better to me since other format values
could be implemented, like "global.verbose = standard"...

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Dragan Simic]

On 2023-10-28 04:52, Jacob Stopak wrote:
>> They currently don't exist, but that's something I've planned to 
>> implement,
>> e.g. to "add.verbose" as a new configuration option.  It should be 
>> usable,
>> while not being messy or intrusive as a new feature.
> 
> "git add" already has the -v, --verbose flag available for the command
> itself like:
> 
> $ git add -v foo.txt
> add 'foo.txt'
> 
> But like you said the config option add.verbose doesn't seem to exist 
> yet.
> 
> So I assume an "add.verbose" config option would just always print that
> without having to specify the -v, --verbose flag when running the 
> command?

Yes, that's how I see it.  Setting "add.verbose" to "true", to be 
precise, or to "basic", which I'll explain a bit further later in my 
response.

> Basically what I'm asking is if commands that already have a --verbose 
> flag
> would just get a config setting that does the existing thing by 
> default?

Well, not by default.  The default values would remain "false", so 
nothing jumps out of nowhere.

>>> So it seems like currently "verbose" is used for various things among
>>> the command set...
>> 
>> Yes, that's the basic verbosity, as I named it above.
> 
> Would it make sense to try to define a more consistent type of output 
> or
> format style for "verbosity" across different commands? As it stands it
> seems each command treats verbosity in its own way which makes it hard 
> to
> interpret exactly what it will do each time...

We'd have to follow the already established behavior of the commands, 
and there are the man pages to describe what's going on with the 
verbosity for each command.  In other words, nothing would get changed, 
just some more knobs would be added, for those who prefer to have the 
additional verbosity enabled.

>>> Another thing is that commands like status have multiple flags that 
>>> can be
>>> used to specify the output format, such as --short, --long, 
>>> --porcelain,
>>> etc, but only --short seems to be configurable as a git config 
>>> setting.
>>> Is there a reason (besides backward compatibility I guess) that these
>>> aren't rolled into a single thing like --format=<type>? This seems 
>>> like
>>> it would be the easiest way to future proof for new formats like
>>> --format=verbose, --format=noob, --format=extended, etc.
>> 
>> That's a good question, but I'd need to go through the commit history 
>> to be
>> able to provide some kind of an explanation.  It could also be all 
>> packed
>> into "status.verbose" as a new configuration option.
> 
> Ok so it sounds like you prefer to use "verbose" as the setting key?
> I guess at this point that might make more sense since commit.verbose
> already exists, and existing options could be packed into it like you
> said instead of just true or false.

It looks like a logical choice to me.

> And then my thing here would just be called "command.verbose = 
> extended"?

Yes, that's what I propose.  It also looks like a logical choice to me, 
and it would leave space for some possible later changes to the 
"<command>.verbose = extended" verbosity, without tying it to the 
tables.  We'd also leave some space that way for even maybe an 
additional level of verbosity, be it "<command>.verbose = simple", 
"<command>.verbose = graphical" or whatever.

Perhaps this scheme should also support "<command>.verbose = basic", 
which would be an alias for "<command>.verbose = true", for additional 
clarity.

>>> From a noob's perspective though, does adding a config setting for 
>>> each
>>> command really make sense? I'm kindof envisioning this setting now as 
>>> a
>>> "mode" that is either enabled for all commands it affects or for 
>>> none.
>>> And it's highly unlikely a newish user would individually discover 
>>> which
>>> commands this "extended" format is available for, and run "git config
>>> <command>.verbose = extended" for every one. I mean we could do that
>>> in case there are folks who only want it for specific commands, but 
>>> to
>>> fulfill it's purpose I think there should definetely be some general 
>>> way
>>> to enable the setting for all commands that have it.
>> 
>> Quite frankly, we shouldn't expect that all users are noobs, and as a 
>> result
>> dumb everything down just to make them as comfortable as possible.  On 
>> the
>> other hand, perhaps not everyone would like to have extended verbosity
>> enabled for all commands, just as not everyone uses "-v" for all 
>> commands.
> 
> I agree with this, and I think it's important to cater to both newbies 
> and
> experienced users alike. That's why I said I never dreamed of making 
> this
> new format the default.

Perhaps it would also be good to nudge the newbies a bit by requesting 
them to enable the extended verbosity for each command by hand.  That 
way they would both learn a bit about the way git configuration works, 
which they ultimately can't escape from, and they would be excited to 
learn new git commands.  Or I at least hope so. :)

> And it's true that some users might only want the extended (or any 
> format)
> for specific commands. I think a happy medium then is to have the 
> command-
> specific settings like you mention, plus one toplevel option that 
> enables a
> specific type of output format among all commands (and overrides the
> command-specific settings), so that the user can choose which they 
> prefer.

That's something we can consider as an additional configuration option.  
That way, users could also enable the basic verbosity for all commands, 
which may also be usable.

> Any thoughts on what the section in the config for a more general 
> setting
> like this might be named? If "status.verbose = extended" would already 
> be
> taken specifically for the status command, what terminology could we 
> use
> to mean something like "global.verbose = extended" or "global.extended 
> =
> true"? Although the former seems better to me since other format values
> could be implemented, like "global.verbose = standard"...

Maybe "core.verbose"?  We already have "core.pager", which kind of 
affects the way all command outputs look like.

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Jacob Stopak]

On Sat, Oct 28, 2023 at 07:55:31AM +0200, Dragan Simic wrote:
> > So I assume an "add.verbose" config option would just always print that
> > without having to specify the -v, --verbose flag when running the
> > command?
> 
> Yes, that's how I see it.  Setting "add.verbose" to "true", to be precise,
> or to "basic", which I'll explain a bit further later in my response.

Ok, gotcha!

> > Basically what I'm asking is if commands that already have a --verbose
> > flag
> > would just get a config setting that does the existing thing by default?
> 
> Well, not by default.  The default values would remain "false", so nothing
> jumps out of nowhere.

Right, sorry, I worded that poorly.

> > > > So it seems like currently "verbose" is used for various things among
> > > > the command set...
> > > 
> > > Yes, that's the basic verbosity, as I named it above.

Ok.

> > Would it make sense to try to define a more consistent type of output or
> > format style for "verbosity" across different commands? As it stands it
> > seems each command treats verbosity in its own way which makes it hard
> > to interpret exactly what it will do each time...
> 
> We'd have to follow the already established behavior of the commands, and
> there are the man pages to describe what's going on with the verbosity for
> each command.  In other words, nothing would get changed, just some more
> knobs would be added, for those who prefer to have the additional verbosity
> enabled.

Ok I see.

> > Ok so it sounds like you prefer to use "verbose" as the setting key?
> > I guess at this point that might make more sense since commit.verbose
> > already exists, and existing options could be packed into it like you
> > said instead of just true or false.
> 
> It looks like a logical choice to me.
> 

Ok.

> > And then my thing here would just be called "command.verbose =
> > extended"?
> 
> Yes, that's what I propose.  It also looks like a logical choice to me, and
> it would leave space for some possible later changes to the
> "<command>.verbose = extended" verbosity, without tying it to the tables.
> We'd also leave some space that way for even maybe an additional level of
> verbosity, be it "<command>.verbose = simple", "<command>.verbose =
> graphical" or whatever.
> 
> Perhaps this scheme should also support "<command>.verbose = basic", which
> would be an alias for "<command>.verbose = true", for additional clarity.
> 

Sounds good!

> 
> Perhaps it would also be good to nudge the newbies a bit by requesting them
> to enable the extended verbosity for each command by hand.  That way they
> would both learn a bit about the way git configuration works, which they
> ultimately can't escape from, and they would be excited to learn new git
> commands.  Or I at least hope so. :)
> 

Hehe ok, maybe there is room in some hints to notify users of the
extended option...

> > And it's true that some users might only want the extended (or any
> > format) for specific commands. I think a happy medium then is to have
> > the command-specific settings like you mention, plus one toplevel
> > option that enables a specific type of output format among all commands
> > (and overrides the command-specific settings), so that the user can
> > choose which they prefer.
> 
> That's something we can consider as an additional configuration option.
> That way, users could also enable the basic verbosity for all commands,
> which may also be usable.
> 

Cool!

> > Any thoughts on what the section in the config for a more general
> > setting like this might be named?
> 
> Maybe "core.verbose"?  We already have "core.pager", which kind of affects
> the way all command outputs look like.

Ok! The idea of using "core" came to mind but I wasn't sure if that was
more for lower-level settings or more general things.

Great. Thanks a lot for all the feedback. Let me doctor up the patch
series to take these things into account and submit an RFC v3 :D

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Dragan Simic]

On 2023-10-28 17:21, Jacob Stopak wrote:
> On Sat, Oct 28, 2023 at 07:55:31AM +0200, Dragan Simic wrote:
>>> Basically what I'm asking is if commands that already have a 
>>> --verbose
>>> flag would just get a config setting that does the existing thing by
>>> default?
>> 
>> Well, not by default.  The default values would remain "false", so 
>> nothing
>> jumps out of nowhere.
> 
> Right, sorry, I worded that poorly.

No worries, just wanted to make sure we're on the same page.

>> Yes, that's what I propose.  It also looks like a logical choice to 
>> me, and
>> it would leave space for some possible later changes to the
>> "<command>.verbose = extended" verbosity, without tying it to the 
>> tables.
>> We'd also leave some space that way for even maybe an additional level 
>> of
>> verbosity, be it "<command>.verbose = simple", "<command>.verbose =
>> graphical" or whatever.
>> 
>> Perhaps this scheme should also support "<command>.verbose = basic", 
>> which
>> would be an alias for "<command>.verbose = true", for additional 
>> clarity.
>> 
> Sounds good!

I'm glad you agree.

>> Perhaps it would also be good to nudge the newbies a bit by requesting 
>> them
>> to enable the extended verbosity for each command by hand.  That way 
>> they
>> would both learn a bit about the way git configuration works, which 
>> they
>> ultimately can't escape from, and they would be excited to learn new 
>> git
>> commands.  Or I at least hope so. :)
>> 
> Hehe ok, maybe there is room in some hints to notify users of the
> extended option...

I agree, there should be a well-placed hint, but we'd need to think 
really well where to place it, so we don't annoy experienced git users 
too much, while we also inform the less experienced users.

>> > Any thoughts on what the section in the config for a more general
>> > setting like this might be named?
>> 
>> Maybe "core.verbose"?  We already have "core.pager", which kind of 
>> affects
>> the way all command outputs look like.
> 
> Ok! The idea of using "core" came to mind but I wasn't sure if that was
> more for lower-level settings or more general things.

I also considered the "core" section to be reserved for the very 
low-level internal things, but having "core.pager" clearly allows other 
appropriate configuration options to be placed here.

> Great. Thanks a lot for all the feedback. Let me doctor up the patch
> series to take these things into account and submit an RFC v3 :D

Sounds good, thank you.  If you agree, I'll go ahead and implement 
support for a few "<command>.verbose" configuration options during the 
next week or so, and submit the patches.  I'll most probably come to 
some more important conclusions while implementing that, which I'll 
relay over, of course.

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Jacob Stopak]

On Sat, Oct 28, 2023 at 06:20:41PM +0200, Dragan Simic wrote:
> > Hehe ok, maybe there is room in some hints to notify users of the
> > extended option...
> 
> I agree, there should be a well-placed hint, but we'd need to think really
> well where to place it, so we don't annoy experienced git users too much,
> while we also inform the less experienced users.

Yes, hmm, I wonder if maybe we could add the hint for the extended option
only in the case that the user uses the --verbose option either on the
command line or via the config setting. Since using the verbose option
tells us the user is asking for more details, that might be a good time
to inform them of the _even more detailed_ option, but of course that
hint could be disabled easily if they preferred the "basic" verbosity.

> > > > Any thoughts on what the section in the config for a more general
> > > > setting like this might be named?
> > > 
> > > Maybe "core.verbose"?  We already have "core.pager", which kind of
> > > affects the way all command outputs look like.
> > 
> > Ok! The idea of using "core" came to mind but I wasn't sure if that was
> > more for lower-level settings or more general things.
> 
> I also considered the "core" section to be reserved for the very low-level
> internal things, but having "core.pager" clearly allows other appropriate
> configuration options to be placed here.

Ok awesome!

> > Great. Thanks a lot for all the feedback. Let me doctor up the patch
> > series to take these things into account and submit an RFC v3 :D
> 
> Sounds good, thank you.  If you agree, I'll go ahead and implement support
> for a few "<command>.verbose" configuration options during the next week or
> so, and submit the patches.  I'll most probably come to some more important
> conclusions while implementing that, which I'll relay over, of course.

Yes I agree, that sounds great! Maybe I'll just wait then until seeing
your implementation of that before I poke around on mine more. Then I'll
apply your patches locally to add my extended option.

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Dragan Simic]

On 2023-10-28 19:35, Jacob Stopak wrote:
> On Sat, Oct 28, 2023 at 06:20:41PM +0200, Dragan Simic wrote:
>> I agree, there should be a well-placed hint, but we'd need to think 
>> really
>> well where to place it, so we don't annoy experienced git users too 
>> much,
>> while we also inform the less experienced users.
> 
> Yes, hmm, I wonder if maybe we could add the hint for the extended 
> option
> only in the case that the user uses the --verbose option either on the
> command line or via the config setting. Since using the verbose option
> tells us the user is asking for more details, that might be a good time
> to inform them of the _even more detailed_ option, but of course that
> hint could be disabled easily if they preferred the "basic" verbosity.

That's something we can keep thinking about, until we find a good 
solution.  Also, a detailed review of the current logic behind 
displaying the hints should be performed first, if you agree.

>> Sounds good, thank you.  If you agree, I'll go ahead and implement 
>> support
>> for a few "<command>.verbose" configuration options during the next 
>> week or
>> so, and submit the patches.  I'll most probably come to some more 
>> important
>> conclusions while implementing that, which I'll relay over, of course.
> 
> Yes I agree, that sounds great! Maybe I'll just wait then until seeing
> your implementation of that before I poke around on mine more. Then 
> I'll
> apply your patches locally to add my extended option.

Great, thanks.  I'll start working on the patches tomorrow or so, and 
I'll get back with any important conclusions or open questions arising 
from that, so we can discuss them further.

Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore

[Jacob Stopak]

On Sat, Oct 28, 2023 at 07:41:47PM +0200, Dragan Simic wrote:
> That's something we can keep thinking about, until we find a good solution.
> Also, a detailed review of the current logic behind displaying the hints
> should be performed first, if you agree.

Yes of course.

> > Yes I agree, that sounds great! Maybe I'll just wait then until seeing
> > your implementation of that before I poke around on mine more. Then I'll
> > apply your patches locally to add my extended option.
> 
> Great, thanks.  I'll start working on the patches tomorrow or so, and I'll
> get back with any important conclusions or open questions arising from that,
> so we can discuss them further.

:)