Git development
 help / color / mirror / Atom feed
* Re: Making it possible to do “git push origin” instead of “git push origin <branch>”, without having to one-time prepare each branch for it
From: Stefan Monov @ 2016-12-22 21:18 UTC (permalink / raw)
  To: git
In-Reply-To: <CAJtFkWtjowyGaFfsCVd-HAZM2-3e0=CkkyYfxne8KRdYq5kJ9g@mail.gmail.com>

Also, if I do the "setup" step (`push -u`) for a branch that doesn't
exist yet (neither on my PC nor on the server), does that remove the
need to do `git checkout -b <branch>` first?

On Thu, Dec 22, 2016 at 11:14 PM, Stefan Monov <logixoul@gmail.com> wrote:
> Hi.
>
> I'd like to use just:
>
>     git push
>
> or at most:
>
>     git push origin
>
> rather than having to first check which is the active branch with `git
> branch --list`, then type:
>
>     git push origin <branch>
>
> At [1] and [2] I've seen that if I do this once:
>
>     git push -u origin <branch>
>
> then from then on I can use just `git push` _for that branch_.
> However, I don't want to do this "setup" step for each branch, because
> it's extra work that I also may forget to do.
>
> Why is this "setup" step necessary and can I avoid it?
>
> Thanks,
> Stefan
>
> [1] http://stackoverflow.com/q/19312622
> [2] http://stackoverflow.com/q/6529136

^ permalink raw reply
* Making it possible to do “git push origin” instead of “git push origin <branch>”, without having to one-time prepare each branch for it
From: Stefan Monov @ 2016-12-22 21:14 UTC (permalink / raw)
  To: git

Hi.

I'd like to use just:

    git push

or at most:

    git push origin

rather than having to first check which is the active branch with `git
branch --list`, then type:

    git push origin <branch>

At [1] and [2] I've seen that if I do this once:

    git push -u origin <branch>

then from then on I can use just `git push` _for that branch_.
However, I don't want to do this "setup" step for each branch, because
it's extra work that I also may forget to do.

Why is this "setup" step necessary and can I avoid it?

Thanks,
Stefan

[1] http://stackoverflow.com/q/19312622
[2] http://stackoverflow.com/q/6529136

^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Junio C Hamano @ 2016-12-22 21:12 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Jacob Keller, Git mailing list, Norbert Kiesel
In-Reply-To: <CA+P7+xp=7h7oATwO6vunqO+nfGhvQgiRkwG0P44hC4YLW2MRhA@mail.gmail.com>

Jacob Keller <jacob.keller@gmail.com> writes:

> I don't think we have too many config options that interact in this
> way, so I understand that "last writing of a particular configuration"
> makes sense, but interactions between configs is something that would
> have never occurred to me. I'll send a patch to drop the compaction
> heuristic since I think we're all 100% in agreement that it is
> superseded by the new configuration (as no case has been shown where
> the new one is worse than compaction, and most show it to be better).

If I recall correctly, we agreed that we'll drop the implementation
of compaction, but use the name --compaction-heuristics to trigger
the new and improved "indent heuristics":

    <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net>

So let's do this.

-- >8 --
Subject: [PATCH] diff: retire the original experimental "compaction" heuristics

This retires the experimental "compaction" heuristics but with a
twist.  It removes the mention of "indent" heuristics, which was a
competing experiment, from everywhere, guts the core logic of the
original "compaction" heuristics out and replaces it with the logic
used by the "indent" heuristics.

The externally visible effect of this change is that people who have
been experimenting by setting diff.compactionHeuristic configuration
or giving the command line option --compaction-heuristic will start
getting the behaviour based on the improved heuristics that used to
be called "indent" heuristics.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/diff-config.txt            |  6 ++---
 Documentation/diff-heuristic-options.txt |  2 --
 builtin/blame.c                          |  3 +--
 diff.c                                   | 25 ++++-----------------
 git-add--interactive.perl                |  5 +----
 t/t4061-diff-indent.sh                   | 38 ++++++++++++++++----------------
 xdiff/xdiff.h                            |  1 -
 xdiff/xdiffi.c                           | 37 +++----------------------------
 8 files changed, 30 insertions(+), 87 deletions(-)

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 58f4bd6afa..39fff3aef9 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -171,11 +171,9 @@ diff.tool::
 
 include::mergetools-diff.txt[]
 
-diff.indentHeuristic::
 diff.compactionHeuristic::
-	Set one of these options to `true` to enable one of two
-	experimental heuristics that shift diff hunk boundaries to
-	make patches easier to read.
+	Set this option to `true` to enable experimental heuristics
+	that shift diff hunk boundaries to make patches easier to read.
 
 diff.algorithm::
 	Choose a diff algorithm.  The variants are as follows:
diff --git a/Documentation/diff-heuristic-options.txt b/Documentation/diff-heuristic-options.txt
index 36cb549df9..3cb024aa22 100644
--- a/Documentation/diff-heuristic-options.txt
+++ b/Documentation/diff-heuristic-options.txt
@@ -1,5 +1,3 @@
---indent-heuristic::
---no-indent-heuristic::
 --compaction-heuristic::
 --no-compaction-heuristic::
 	These are to help debugging and tuning experimental heuristics
diff --git a/builtin/blame.c b/builtin/blame.c
index 4ddfadb71f..395d4011fb 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2596,7 +2596,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		 * and are only included here to get included in the "-h"
 		 * output:
 		 */
-		{ OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental indent-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
 		{ OPTION_LOWLEVEL_CALLBACK, 0, "compaction-heuristic", NULL, NULL, N_("Use an experimental blank-line-based heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
 
 		OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find better match"), XDF_NEED_MINIMAL),
@@ -2645,7 +2644,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	}
 parse_done:
 	no_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);
-	xdl_opts |= revs.diffopt.xdl_opts & (XDF_COMPACTION_HEURISTIC | XDF_INDENT_HEURISTIC);
+	xdl_opts |= revs.diffopt.xdl_opts & XDF_COMPACTION_HEURISTIC;
 	DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
 	argc = parse_options_end(&ctx);
 
diff --git a/diff.c b/diff.c
index 8981477c43..f1b01f5b1e 100644
--- a/diff.c
+++ b/diff.c
@@ -27,7 +27,6 @@
 #endif
 
 static int diff_detect_rename_default;
-static int diff_indent_heuristic; /* experimental */
 static int diff_compaction_heuristic; /* experimental */
 static int diff_rename_limit_default = 400;
 static int diff_suppress_blank_empty;
@@ -223,16 +222,8 @@ void init_diff_ui_defaults(void)
 
 int git_diff_heuristic_config(const char *var, const char *value, void *cb)
 {
-	if (!strcmp(var, "diff.indentheuristic")) {
-		diff_indent_heuristic = git_config_bool(var, value);
-		if (diff_indent_heuristic)
-			diff_compaction_heuristic = 0;
-	}
-	if (!strcmp(var, "diff.compactionheuristic")) {
+	if (!strcmp(var, "diff.compactionheuristic"))
 		diff_compaction_heuristic = git_config_bool(var, value);
-		if (diff_compaction_heuristic)
-			diff_indent_heuristic = 0;
-	}
 	return 0;
 }
 
@@ -3378,9 +3369,7 @@ void diff_setup(struct diff_options *options)
 	options->use_color = diff_use_color_default;
 	options->detect_rename = diff_detect_rename_default;
 	options->xdl_opts |= diff_algorithm;
-	if (diff_indent_heuristic)
-		DIFF_XDL_SET(options, INDENT_HEURISTIC);
-	else if (diff_compaction_heuristic)
+	if (diff_compaction_heuristic)
 		DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
 
 	options->orderfile = diff_order_file_cfg;
@@ -3876,15 +3865,9 @@ int diff_opt_parse(struct diff_options *options,
 		DIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL);
 	else if (!strcmp(arg, "--ignore-blank-lines"))
 		DIFF_XDL_SET(options, IGNORE_BLANK_LINES);
-	else if (!strcmp(arg, "--indent-heuristic")) {
-		DIFF_XDL_SET(options, INDENT_HEURISTIC);
-		DIFF_XDL_CLR(options, COMPACTION_HEURISTIC);
-	} else if (!strcmp(arg, "--no-indent-heuristic"))
-		DIFF_XDL_CLR(options, INDENT_HEURISTIC);
-	else if (!strcmp(arg, "--compaction-heuristic")) {
+	else if (!strcmp(arg, "--compaction-heuristic"))
 		DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
-		DIFF_XDL_CLR(options, INDENT_HEURISTIC);
-	} else if (!strcmp(arg, "--no-compaction-heuristic"))
+	else if (!strcmp(arg, "--no-compaction-heuristic"))
 		DIFF_XDL_CLR(options, COMPACTION_HEURISTIC);
 	else if (!strcmp(arg, "--patience"))
 		options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index ee3d812695..642cce1ac6 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -45,7 +45,6 @@
 my $normal_color = $repo->get_color("", "reset");
 
 my $diff_algorithm = $repo->config('diff.algorithm');
-my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');
 my $diff_compaction_heuristic = $repo->config_bool('diff.compactionheuristic');
 my $diff_filter = $repo->config('interactive.difffilter');
 
@@ -751,9 +750,7 @@ sub parse_diff {
 	if (defined $diff_algorithm) {
 		splice @diff_cmd, 1, 0, "--diff-algorithm=${diff_algorithm}";
 	}
-	if ($diff_indent_heuristic) {
-		splice @diff_cmd, 1, 0, "--indent-heuristic";
-	} elsif ($diff_compaction_heuristic) {
+	if ($diff_compaction_heuristic) {
 		splice @diff_cmd, 1, 0, "--compaction-heuristic";
 	}
 	if (defined $patch_mode_revision) {
diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index 556450609b..30f809d0d3 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='Test diff indent heuristic.
+test_description='Test diff compaction heuristic.
 
 '
 . ./test-lib.sh
@@ -157,28 +157,28 @@ test_expect_success 'diff: ugly spaces' '
 	compare_diff spaces-expect out
 '
 
-test_expect_success 'diff: nice spaces with --indent-heuristic' '
-	git diff --indent-heuristic old new -- spaces.txt >out-compacted &&
+test_expect_success 'diff: nice spaces with --compaction-heuristic' '
+	git diff --compaction-heuristic old new -- spaces.txt >out-compacted &&
 	compare_diff spaces-compacted-expect out-compacted
 '
 
-test_expect_success 'diff: nice spaces with diff.indentHeuristic' '
-	git -c diff.indentHeuristic=true diff old new -- spaces.txt >out-compacted2 &&
+test_expect_success 'diff: nice spaces with diff.compactionHeuristic' '
+	git -c diff.compactionHeuristic=true diff old new -- spaces.txt >out-compacted2 &&
 	compare_diff spaces-compacted-expect out-compacted2
 '
 
-test_expect_success 'diff: --no-indent-heuristic overrides config' '
-	git -c diff.indentHeuristic=true diff --no-indent-heuristic old new -- spaces.txt >out2 &&
+test_expect_success 'diff: --no-compaction-heuristic overrides config' '
+	git -c diff.compactionHeuristic=true diff --no-compaction-heuristic old new -- spaces.txt >out2 &&
 	compare_diff spaces-expect out2
 '
 
-test_expect_success 'diff: --indent-heuristic with --patience' '
-	git diff --indent-heuristic --patience old new -- spaces.txt >out-compacted3 &&
+test_expect_success 'diff: --compaction-heuristic with --patience' '
+	git diff --compaction-heuristic --patience old new -- spaces.txt >out-compacted3 &&
 	compare_diff spaces-compacted-expect out-compacted3
 '
 
-test_expect_success 'diff: --indent-heuristic with --histogram' '
-	git diff --indent-heuristic --histogram old new -- spaces.txt >out-compacted4 &&
+test_expect_success 'diff: --compaction-heuristic with --histogram' '
+	git diff --compaction-heuristic --histogram old new -- spaces.txt >out-compacted4 &&
 	compare_diff spaces-compacted-expect out-compacted4
 '
 
@@ -187,8 +187,8 @@ test_expect_success 'diff: ugly functions' '
 	compare_diff functions-expect out
 '
 
-test_expect_success 'diff: nice functions with --indent-heuristic' '
-	git diff --indent-heuristic old new -- functions.c >out-compacted &&
+test_expect_success 'diff: nice functions with --compaction-heuristic' '
+	git diff --compaction-heuristic old new -- functions.c >out-compacted &&
 	compare_diff functions-compacted-expect out-compacted
 '
 
@@ -197,18 +197,18 @@ test_expect_success 'blame: ugly spaces' '
 	compare_blame spaces-expect out-blame
 '
 
-test_expect_success 'blame: nice spaces with --indent-heuristic' '
-	git blame --indent-heuristic old..new -- spaces.txt >out-blame-compacted &&
+test_expect_success 'blame: nice spaces with --compaction-heuristic' '
+	git blame --compaction-heuristic old..new -- spaces.txt >out-blame-compacted &&
 	compare_blame spaces-compacted-expect out-blame-compacted
 '
 
-test_expect_success 'blame: nice spaces with diff.indentHeuristic' '
-	git -c diff.indentHeuristic=true blame old..new -- spaces.txt >out-blame-compacted2 &&
+test_expect_success 'blame: nice spaces with diff.compactionHeuristic' '
+	git -c diff.compactionHeuristic=true blame old..new -- spaces.txt >out-blame-compacted2 &&
 	compare_blame spaces-compacted-expect out-blame-compacted2
 '
 
-test_expect_success 'blame: --no-indent-heuristic overrides config' '
-	git -c diff.indentHeuristic=true blame --no-indent-heuristic old..new -- spaces.txt >out-blame2 &&
+test_expect_success 'blame: --no-compaction-heuristic overrides config' '
+	git -c diff.compactionHeuristic=true blame --no-compaction-heuristic old..new -- spaces.txt >out-blame2 &&
 	git blame old..new -- spaces.txt >out-blame &&
 	compare_blame spaces-expect out-blame2
 '
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 8db16d4ae6..7423f77fc8 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -42,7 +42,6 @@ extern "C" {
 #define XDF_IGNORE_BLANK_LINES (1 << 7)
 
 #define XDF_COMPACTION_HEURISTIC (1 << 8)
-#define XDF_INDENT_HEURISTIC (1 << 9)
 
 #define XDL_EMIT_FUNCNAMES (1 << 0)
 #define XDL_EMIT_FUNCCONTEXT (1 << 2)
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 760fbb6db7..2131ea4920 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -400,11 +400,6 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,
 }
 
 
-static int is_blank_line(xrecord_t *rec, long flags)
-{
-	return xdl_blankline(rec->ptr, rec->size, flags);
-}
-
 static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags)
 {
 	return (rec1->ha == rec2->ha &&
@@ -821,7 +816,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 	struct xdlgroup g, go;
 	long earliest_end, end_matching_other;
 	long groupsize;
-	unsigned int blank_lines;
 
 	group_init(xdf, &g);
 	group_init(xdfo, &go);
@@ -846,13 +840,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 			 */
 			end_matching_other = -1;
 
-			/*
-			 * Boolean value that records whether there are any blank
-			 * lines that could be made to be the last line of this
-			 * group.
-			 */
-			blank_lines = 0;
-
 			/* Shift the group backward as much as possible: */
 			while (!group_slide_up(xdf, &g, flags))
 				if (group_previous(xdfo, &go))
@@ -869,11 +856,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 
 			/* Now shift the group forward as far as possible: */
 			while (1) {
-				if (!blank_lines)
-					blank_lines = is_blank_line(
-							xdf->recs[g.end - 1],
-							flags);
-
 				if (group_slide_down(xdf, &g, flags))
 					break;
 				if (group_next(xdfo, &go))
@@ -906,24 +888,11 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 				if (group_previous(xdfo, &go))
 					xdl_bug("group sync broken sliding to match");
 			}
-		} else if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) {
+		} else if (flags & XDF_COMPACTION_HEURISTIC) {
 			/*
-			 * Compaction heuristic: if it is possible to shift the
-			 * group to make its bottom line a blank line, do so.
+			 * Heuristic based on the indentation level.
 			 *
-			 * As we already shifted the group forward as far as
-			 * possible in the earlier loop, we only need to handle
-			 * backward shifts, not forward ones.
-			 */
-			while (!is_blank_line(xdf->recs[g.end - 1], flags)) {
-				if (group_slide_up(xdf, &g, flags))
-					xdl_bug("blank line disappeared");
-				if (group_previous(xdfo, &go))
-					xdl_bug("group sync broken sliding to blank line");
-			}
-		} else if (flags & XDF_INDENT_HEURISTIC) {
-			/*
-			 * Indent heuristic: a group of pure add/delete lines
+			 * A group of pure add/delete lines
 			 * implies two splits, one between the end of the "before"
 			 * context and the start of the group, and another between
 			 * the end of the group and the beginning of the "after"
-- 
2.11.0-448-g9a11f8a62b


^ permalink raw reply related
* Re: [PATCH v2 3/3] mingw: replace isatty() hack
From: Johannes Sixt @ 2016-12-22 20:26 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Jeff Hostetler, Junio C Hamano, Pranit Bauva, Beat Bolli
In-Reply-To: <5e3c505a206a735e6ba0bfaf4c73965e95a928eb.1482426497.git.johannes.schindelin@gmx.de>

I've only one request for clarification below. Otherwise, the patch 
looks good.

(lines removed by the patch trimmed)

Am 22.12.2016 um 18:09 schrieb Johannes Schindelin:
> +static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
> +{
> +	/*
> +	 * Create a copy of the original handle associated with fd
> +	 * because the original will get closed when we dup2().
> +	 */
> +	HANDLE handle = (HANDLE)_get_osfhandle(fd);
> +	HANDLE duplicate = duplicate_handle(handle);
>
> +	/* Create a temp fd associated with the already open "new_handle". */
> +	int new_fd = _open_osfhandle((intptr_t)new_handle, O_BINARY);
>
> +	assert((fd == 1) || (fd == 2));
>
> +	/*
> +	 * Use stock dup2() to re-bind fd to the new handle.  Note that
> +	 * this will implicitly close(1) and close both fd=1 and the
> +	 * originally associated handle.  It will open a new fd=1 and
> +	 * call DuplicateHandle() on the handle associated with new_fd.
> +	 * It is because of this implicit close() that we created the
> +	 * copy of the original.
> +	 *
> +	 * Note that the OS can recycle HANDLE (numbers) just like it
> +	 * recycles fd (numbers), so we must update the cached value
> +	 * of "console".  You can use GetFileType() to see that
> +	 * handle and _get_osfhandle(fd) may have the same number
> +	 * value, but they refer to different actual files now.

Certainly, the OS does not recycle handle values that are in use (open). 
Then I do not quite get the point of this paragraph. See...

> +	 *
> +	 * Note that dup2() when given target := {0,1,2} will also
> +	 * call SetStdHandle(), so we don't need to worry about that.
> +	 */
> +	dup2(new_fd, fd);
> +	if (console == handle)
> +		console = duplicate;

... This is where "the cached value of console is updated", right? If 
console == handle, then this is not because a handle value was recycled, 
but because fd *was* console. Since the old value of console has been 
closed by the dup2(), we must refer to the back-up value in the future. 
Am I missing something?

> +	handle = INVALID_HANDLE_VALUE;
>
> +	/* Close the temp fd.  This explicitly closes "new_handle"
> +	 * (because it has been associated with it).
> +	 */
> +	close(new_fd);
>
> +	fd_is_interactive[fd] |= FD_SWAPPED;
>
> +	return duplicate;
>  }


^ permalink raw reply
* Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
From: Junio C Hamano @ 2016-12-22 19:33 UTC (permalink / raw)
  To: Brandon Williams
  Cc: Johannes Sixt, git, sbeller, peff, jacob.keller, ramsay, tboegi,
	pclouds
In-Reply-To: <20161222173301.GB119874@google.com>

Brandon Williams <bmwill@google.com> writes:

> On 12/22, Johannes Sixt wrote:
>> Am 21.12.2016 um 23:33 schrieb Brandon Williams:
>> >On 12/21, Johannes Sixt wrote:
>> >>+/* copies root part from remaining to resolved, canonicalizing it on the way */
>> >>+static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
>> >>+{
>> >>+	int offset = offset_1st_component(remaining->buf);
>> >>+
>> >>+	strbuf_reset(resolved);
>> >>+	strbuf_add(resolved, remaining->buf, offset);
>> >>+#ifdef GIT_WINDOWS_NATIVE
>> >>+	convert_slashes(resolved->buf);
>> >>+#endif
>> >
>> >So then the only extra cononicalization that is happening here is
>> >converting '\\server\share' to '//server/share'? (or 'c:\' to 'c:/')
>> 
>> Correct. All other directory separators are canonicalized by the
>> primary function, strbuf_realpath.
>
> Sounds good. Logically everything looks good to me.  And I like that
> setting 'resolved' to the root of an abs path is pulled out into a
> helper function.  It took me a couple extra seconds to realize that
> offset_1st_component returns 0 with a relative path, which makes causes
> the call to get_root_part to essentially be a noop (ie nothing is
> resolved).
>
> Thanks for helping get this to work on windows!

Thanks, both.  

Let's move the topic with this patch to 'next'.  Further
micro-optimization can be done incrementally if desired.




^ permalink raw reply
* Re: [PATCH] log: support 256 colors with --graph=256colors
From: Junio C Hamano @ 2016-12-22 19:06 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Jeff King, Git Mailing List
In-Reply-To: <CACsJy8CnS1=_vA5xhbZ94Qyh7ySC5FvaALu1vhQwt_YJya4wHA@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

> .... But I think I could approach it a different way:
> collect colors that have names. That reduces the number of colors so
> we can go back to "step 1 at a time" and still don't run into two
> similar colors often.

I suspect that there is a "cultural" bias that makes the idea
unworkable.  I haven't found a definitive source, but I think there
are a lot more hues and shades of red that have names than hues and
shades of blue, for example.

If I were doing this, I'd just prepare a table with 32 color slots
or so [*1*], start at a random spot (say 017:00005f) of

    https://en.wikipedia.org/wiki/File:Xterm_256color_chart.svg, 

and pick spots by jumping southeast like a chess knight
(i.e. 017->030->043->086->...) until the table is filled, wrapping
around at the edge of that color chart as necessary.


[Footnote]

*1* ...because I do not think the thin graph lines painted in too
many colours on the screen would be easily distinguishable from each
other anyway.

^ permalink raw reply
* Re: [PATCH 4/2] t5615-alternate-env: double-quotes in file names do not work on Windows
From: Johannes Sixt @ 2016-12-22 19:06 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <4e66fba1-d881-6a96-1e4d-da9c897353ac@kdbg.org>

Am 22.12.2016 um 07:13 schrieb Johannes Sixt:
> Am 21.12.2016 um 23:42 schrieb Jeff King:
>> Hmph. I explicitly avoided a colon in the filename so that it would run
>> on MINGW. Is a double-quote also not allowed?
>
> It is not allowed; that was my conclusion. But now that you ask, I'll
> double-check.

Ok, the point is that I get this error:

mv: cannot move `one.git' to `"one.git'

I don't feel inclined to find out which of 'mv' or the Windows API or 
the NTFS driver prevents the double-quotes and come up with a 
work-around just to get the test to run in some form. Let's leave it at 
the !MINGW prerequisite.

-- Hannes


^ permalink raw reply
* [RFC/PATCH] add diffstat information to rebase
From: Stefan Beller @ 2016-12-22 18:56 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

Signed-off-by: Stefan Beller <sbeller@google.com>
---

When working on a large feature consisting of lots of commits,
my development workflow is to create a lot of very small commits and
then reshuffle these via interactive rebase.

Sometimes the commit message titles for these very small commits are
not as good as I thought they would, such that the interactive rebase
session needs to be accompanied by extensive use of gitk in my case.

This is a small hack that adds the diffstat to the interactive rebase
helping me a bit during the rebase, such that:

    $ git rebase -i HEAD^^

    pick 2eaa3f532c Third batch for 2.12
    # Documentation/RelNotes/2.12.0.txt | 40 +++++++++++++++++++++++++++++++++++++++
    # 1 file changed, 40 insertions(+)
    pick 3170a3a57b add information to rebase
    # git-rebase--interactive.sh | 2 ++
    # 1 file changed, 2 insertions(+)
    
    # Rebase 2eaa3f532c..3170a3a57b onto 2eaa3f532c (1 command)
    #
    # Commands:
    # p, pick = use commit
    # r, reword = use commit, but edit the commit message
    # e, edit = use commit, but stop for amending

I am not completely satisfied with the result, as I initially wished these
information would just appear in line after the commit subject, but this
comes close. Maybe the last line also needs to be dropped.

This is not a patch meant for inclusion, as for that we'd want to hide this
feature behind an option I'd guess.

Stefan

 git-rebase--interactive.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 41fd374c72..db73c69674 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1220,6 +1220,7 @@ do
 	if test t != "$preserve_merges"
 	then
 		printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+		git diff --stat $sha1^..$sha1 |sed s/^/"$comment_char"/ >>"$todo"
 	else
 		if test -z "$rebase_root"
 		then
@@ -1238,6 +1239,7 @@ do
 		then
 			touch "$rewritten"/$sha1
 			printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+			git diff --stat $sha1^..$sha1 |sed s/^/"$comment_char"/ >>"$todo"
 		fi
 	fi
 done
-- 
2.11.0.193.g3170a3a57b


^ permalink raw reply related
* Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
From: Johannes Sixt @ 2016-12-22 18:54 UTC (permalink / raw)
  To: Brandon Williams
  Cc: git, sbeller, peff, jacob.keller, gitster, ramsay, tboegi,
	pclouds
In-Reply-To: <20161222173301.GB119874@google.com>

Am 22.12.2016 um 18:33 schrieb Brandon Williams:
> It took me a couple extra seconds to realize that
> offset_1st_component returns 0 with a relative path, which makes causes
> the call to get_root_part to essentially be a noop (ie nothing is
> resolved).

Yeah, I am still unsure whether it is a good idea to optimize away the 
is_absolute_path() call, because we lose the symmetry to the symlink 
case, where we cannot get rid of the call...

But I think the condition plus comment

    if (!resolved->len) {
        /* relative path; can use CWD as the initial resolved path */

makes things fairly clear.

-- Hannes


^ permalink raw reply
* Re: [PATCH 0/2] Really fix the isatty() problem on Windows
From: Johannes Sixt @ 2016-12-22 18:48 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, Pranit Bauva
In-Reply-To: <b0541907-ee79-207b-dc0f-1e3e7d761950@kdbg.org>

Am 21.12.2016 um 22:15 schrieb Johannes Sixt:
> Am 21.12.2016 um 18:53 schrieb Johannes Schindelin:
>> The current patch series is based on `pu`, as that already has the
>> winansi_get_osfhandle() fix. For ease of testing, I also have a branch
>> based on master which you can pull via
>>
>>     git pull https://github.com/dscho/git mingw-isatty-fixup-master
>
> Will test and report back tomorrow.

This version 1 of the series passes the test suite (next + a handful 
other topics) for me. It has also undergone a bit of field testing, and 
things look fine.

I haven't looked at the resulting code, yet, but I don't expect to find 
anything fishy.

-- Hannes


^ permalink raw reply
* Re: [PATCH v2 0/3] Really fix the isatty() problem on Windows
From: Junio C Hamano @ 2016-12-22 18:32 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Pranit Bauva, Johannes Sixt, Beat Bolli
In-Reply-To: <alpine.DEB.2.20.1612221852320.155951@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Sorry, but I didn't mean to "suggest replacement".  I was just
>> testing my understanding by attempt to rephrase the gist of it.
>
> I did like your phrasing, though.

OK, then let's use these three patches as-is.  I just made sure that
they can go to maintenance track for 2.11.x; so all should be good.

Thanks.

^ permalink raw reply
* Re: [PATCH v2 0/3] Really fix the isatty() problem on Windows
From: Junio C Hamano @ 2016-12-22 18:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Pranit Bauva, Johannes Sixt, Beat Bolli
In-Reply-To: <xmqqd1gjhn0u.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> ...
>> - fixed the confusing commit message by using Junio's suggested
>>   replacement
>
> Sorry, but I didn't mean to "suggest replacement".  I was just
> testing my understanding by attempt to rephrase the gist of it.
> ...
> Your "use this opportunity to actually clean up" above suggests that
> the answer is the latter, but if you took my "summary of my
> understanding", it is likely that that fact is not captured in the
> resulting log message.

So using the original log message in v1 and what you wrote in the
message I was responding to as references, let me try a real
"suggested" one as penance.

I need to ask one clarification on what you wrote before completing
that effort, though.

>> And incidentally, replacing the previous hack with the clean, new
>> solution, which specifies explicitly for the file descriptors 0, 1 and 2
>> whether we detected an MSYS2 pseudo-tty, whether we detected a real
>> Win32 Console, and whether we had to swap out a real Win32 Console for a
>> pipe to allow child processes to inherit it.

This has subject but not verb.  I parsed the above like so:

    Replacing the previous hack with the clean, new solution (which
    specifies explicitly for the file descriptors 0, 1 and 2
     - whether we detected an MSYS2 pseudo-tty, 
     - whether we detected a real Win32 Console, and 
     - whether we had to swap out a real Win32 Console for a
       pipe to allow child processes to inherit it
    )

So the entire thing is a noun phrase "replacing with a new patch",
and I take that as the subject of an unfinished sentence.  What did
that subject do?  Replacing with a new patch allows us to do "this
wonderful thing", but what "this wonderful thing" is not clear.

Subject: mingw: replace isatty() hack
From: Jeff Hostetler <jeffhost@microsoft.com>

For over a year, Git for Windows has carried a patch that detects
the MSYS2 pseudo ttys used by Git for Windows' default Git Bash
(i.e. a terminal that is not backed by a Win32 Console), but it did
so by accessing internals of a previous MSVC runtime that is no
longer valid in newer versions.  

Clean up this mess by backporting a patch that was originally done
to compile Git with a recent VC++.  Replacing the previous hack with
the clean, new solution, which specifies explicitly for the file
descriptors 0, 1 and 2 whether we detected an MSYS2 pseudo-tty,
whether we detected a real Win32 Console, and whether we had to swap
out a real Win32 Console for a pipe to allow child processes to
inherit it, lets us do XXXXX.

As a side effect (which was the reason for the back-port), this patch
also fixes the previous misguided attempt to intercept isatty() so that
it handles character devices (such as /dev/null) as Git expects it.

^ permalink raw reply
* Re: [PATCH v2 0/3] Really fix the isatty() problem on Windows
From: Johannes Schindelin @ 2016-12-22 17:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Pranit Bauva, Johannes Sixt, Beat Bolli
In-Reply-To: <xmqqd1gjhn0u.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Thu, 22 Dec 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > - unsquashed 2/3 which was improperly snuck in before,
> 
> As Windows specific changes, I didn't notice these two were independent.

Yeah, sorry, I only realized today that I had snuck that one in when
re-reviewing the changes.

> > - fixed the confusing commit message by using Junio's suggested
> >   replacement
> 
> Sorry, but I didn't mean to "suggest replacement".  I was just
> testing my understanding by attempt to rephrase the gist of it.

I did like your phrasing, though.

> There was one thing I still wasn't clear in my "summary of my
> understanding".  Is the "replacement originally done for compiling
> with VC++" a solution that still peeks into MSVC runtime internals
> but is usable with both old and more recent one?  Or is it a more
> kosher approach that does not play with the internals to make it
> unlikely that it would have to change again in the future?

Oh, it is kosher. There is no more messing with internals.

> Your "use this opportunity to actually clean up" above suggests that
> the answer is the latter, but if you took my "summary of my
> understanding", it is likely that that fact is not captured in the
> resulting log message.

Right... I tried to make that clear by saying that this change replaces
the hack.

> The interdiff obviously looks good.  Let's move this series forward.
> I'll see if it can be merged down to 'maint', too, but it probably
> would not matter that much.

I will have to release a new Git for Windows version Real Soon Now [*1*],
so I will have to take those patches kind of there (as Git for Windows
will be based on upstream/maint for a while now).

My thinking is that I will publish a prerelease either tonight or
tomorrow, then go offline until next year, and then immediately publish
Git for Windows v2.11.0(2) (or v2.11.1 if you publish a bugfix version in
the meantime).

Ciao,
Dscho

Footnote *1*: There is a new cURL version with some security fixes,
although I do not think they are super-critical, then there is the fix for
the double slashes in //server/share/directory, the fix for the empty
credentials and I should probably also try to update the MSYS2 runtime to
the newest version of Cygwin's runtime. Lots of stuff.

^ permalink raw reply
* Re: [PATCH] mailinfo.c: move side-effects outside of assert
From: Junio C Hamano @ 2016-12-22 17:57 UTC (permalink / raw)
  To: Jeff King
  Cc: Kyle J. McKay, Johannes Schindelin, Jonathan Tan,
	Git mailing list
In-Reply-To: <20161222033418.dmslmuhq7mqhmkwq@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Well, no, I mostly just said that I do not think there is any point in
> defining NDEBUG in the first place, as there is little or no benefit to
> removing those asserts from the built product.
> ...
> Sure, if you want to mass-convert them, doing so with a macro similar to
> assert is the simplest way. I don't think we are in a huge hurry to do
> that conversion though. I'm not complaining that NDEBUG works as
> advertised by disabling asserts. I'm just claiming that it's largely
> pointless in our code base, and I'd consider die("BUG") to be our
> "usual" style. 

I agree with all of the above. Given the way how our own code uses
assert(), there is little point removing them and turning them over
time into "if (...) die(BUG)" would probably be better.

Borrowed code like nedmalloc may be a different story, but as you
said in a separate message in this thread, I think we are better off
leaving that to those who care about that piece of code.

^ permalink raw reply
* Re: [PATCH v2 0/3] Really fix the isatty() problem on Windows
From: Junio C Hamano @ 2016-12-22 17:49 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Pranit Bauva, Johannes Sixt, Beat Bolli
In-Reply-To: <cover.1482426497.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> My previous fix may have fixed running the new
> t/t6030-bisect-porcelain.sh script that tested the new bisect--helper,
> which in turn used isatty() to determine whether it was running
> interactively and was fooled by being redirected to /dev/null.
>
> But it not only broke paging when running in a CMD window, due to
> testing in the wrong worktree I also missed that it broke paging in Git
> for Windows 2.x' Git Bash (i.e. a MinTTY terminal emulator).
>
> Let's use this opportunity to actually clean up the entire isatty() mess
> once and for all, as part of the problem was introduced by a clever hack
> that messes with internals of the Microsoft C runtime, and which changed
> recently, so it was not such a clever hack to begin with.
>
> Happily, one of my colleagues had to address that latter problem
> recently when he was tasked to make Git compile with Microsoft Visual C
> (the rationale: debugging facilities of Visual Studio are really
> outstanding, try them if you get a chance).
>
> And incidentally, replacing the previous hack with the clean, new
> solution, which specifies explicitly for the file descriptors 0, 1 and 2
> whether we detected an MSYS2 pseudo-tty, whether we detected a real
> Win32 Console, and whether we had to swap out a real Win32 Console for a
> pipe to allow child processes to inherit it.
>
> While at it (or, actually, more like: as I already made this part of v1
> by mistake), upstream the patch carried in Git for Windows that supports
> color when running Git for Windows in Cygwin terminals.
>
> Changes since v1:
>
> - rebased onto master
>
> - unsquashed 2/3 which was improperly snuck in before,

As Windows specific changes, I didn't notice these two were independent.

> - noted that Beat Bolli tested this (see
>   https://github.com/git-for-windows/git/issues/997#issuecomment-268764693)
>
> - fixed the confusing commit message by using Junio's suggested
>   replacement

Sorry, but I didn't mean to "suggest replacement".  I was just
testing my understanding by attempt to rephrase the gist of it.

There was one thing I still wasn't clear in my "summary of my
understanding".  Is the "replacement originally done for compiling
with VC++" a solution that still peeks into MSVC runtime internals
but is usable with both old and more recent one?  Or is it a more
kosher approach that does not play with the internals to make it
unlikely that it would have to change again in the future?

Your "use this opportunity to actually clean up" above suggests that
the answer is the latter, but if you took my "summary of my
understanding", it is likely that that fact is not captured in the
resulting log message.

The interdiff obviously looks good.  Let's move this series forward.
I'll see if it can be merged down to 'maint', too, but it probably
would not matter that much.

Thanks.



^ permalink raw reply
* Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
From: Brandon Williams @ 2016-12-22 17:33 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: git, sbeller, peff, jacob.keller, gitster, ramsay, tboegi,
	pclouds
In-Reply-To: <0c9aa347-d64e-b7d7-9b07-52d844d76252@kdbg.org>

On 12/22, Johannes Sixt wrote:
> Am 21.12.2016 um 23:33 schrieb Brandon Williams:
> >On 12/21, Johannes Sixt wrote:
> >>+/* copies root part from remaining to resolved, canonicalizing it on the way */
> >>+static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
> >>+{
> >>+	int offset = offset_1st_component(remaining->buf);
> >>+
> >>+	strbuf_reset(resolved);
> >>+	strbuf_add(resolved, remaining->buf, offset);
> >>+#ifdef GIT_WINDOWS_NATIVE
> >>+	convert_slashes(resolved->buf);
> >>+#endif
> >
> >So then the only extra cononicalization that is happening here is
> >converting '\\server\share' to '//server/share'? (or 'c:\' to 'c:/')
> 
> Correct. All other directory separators are canonicalized by the
> primary function, strbuf_realpath.

Sounds good. Logically everything looks good to me.  And I like that
setting 'resolved' to the root of an abs path is pulled out into a
helper function.  It took me a couple extra seconds to realize that
offset_1st_component returns 0 with a relative path, which makes causes
the call to get_root_part to essentially be a noop (ie nothing is
resolved).

Thanks for helping get this to work on windows!

-- 
Brandon Williams

^ permalink raw reply
* [PATCH v2 1/3] mingw: adjust is_console() to work with stdin
From: Johannes Schindelin @ 2016-12-22 17:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Pranit Bauva, Johannes Sixt, Beat Bolli
In-Reply-To: <cover.1482426497.git.johannes.schindelin@gmx.de>

When determining whether a handle corresponds to a *real* Win32 Console
(as opposed to, say, a character device such as /dev/null), we use the
GetConsoleOutputBufferInfo() function as a tell-tale.

However, that does not work for *input* handles associated with a
console. Let's just use the GetConsoleMode() function for input handles,
and since it does not work on output handles fall back to the previous
method for those.

This patch prepares for using is_console() instead of my previous
misguided attempt in cbb3f3c9b1 (mingw: intercept isatty() to handle
/dev/null as Git expects it, 2016-12-11) that broke everything on
Windows.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/winansi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index cb725fb02f..590d61cb1b 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -84,6 +84,7 @@ static void warn_if_raster_font(void)
 static int is_console(int fd)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
+	DWORD mode;
 	HANDLE hcon;
 
 	static int initialized = 0;
@@ -98,7 +99,10 @@ static int is_console(int fd)
 		return 0;
 
 	/* check if its a handle to a console output screen buffer */
-	if (!GetConsoleScreenBufferInfo(hcon, &sbi))
+	if (!fd) {
+		if (!GetConsoleMode(hcon, &mode))
+			return 0;
+	} else if (!GetConsoleScreenBufferInfo(hcon, &sbi))
 		return 0;
 
 	/* initialize attributes */
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related
* [PATCH v2 3/3] mingw: replace isatty() hack
From: Johannes Schindelin @ 2016-12-22 17:09 UTC (permalink / raw)
  To: git; +Cc: Jeff Hostetler, Junio C Hamano, Pranit Bauva, Johannes Sixt,
	Beat Bolli
In-Reply-To: <cover.1482426497.git.johannes.schindelin@gmx.de>

From: Jeff Hostetler <jeffhost@microsoft.com>

Git for Windows has carried a patch that depended on internals
of MSVC runtime, but it does not work correctly with recent MSVC
runtime. A replacement was written originally for compiling
with VC++. The patch in this message is a backport of that
replacement, and it also fixes the previous attempt to make
isatty() tell that /dev/null is *not* an interactive terminal.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Tested-by: Beat Bolli <dev+git@drbeat.li>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/winansi.c | 176 ++++++++++++++++++++++---------------------------------
 1 file changed, 69 insertions(+), 107 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index fa37695fca..477209fce7 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -6,9 +6,12 @@
 #include "../git-compat-util.h"
 #include <wingdi.h>
 #include <winreg.h>
+#include "win32.h"
 
-/* In this file, we actually want to use Windows' own isatty(). */
-#undef isatty
+static int fd_is_interactive[3] = { 0, 0, 0 };
+#define FD_CONSOLE 0x1
+#define FD_SWAPPED 0x2
+#define FD_MSYS    0x4
 
 /*
  ANSI codes used by git: m, K
@@ -105,6 +108,9 @@ static int is_console(int fd)
 	} else if (!GetConsoleScreenBufferInfo(hcon, &sbi))
 		return 0;
 
+	if (fd >= 0 && fd <= 2)
+		fd_is_interactive[fd] |= FD_CONSOLE;
+
 	/* initialize attributes */
 	if (!initialized) {
 		console = hcon;
@@ -466,76 +472,50 @@ static HANDLE duplicate_handle(HANDLE hnd)
 	return hresult;
 }
 
+static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
+{
+	/*
+	 * Create a copy of the original handle associated with fd
+	 * because the original will get closed when we dup2().
+	 */
+	HANDLE handle = (HANDLE)_get_osfhandle(fd);
+	HANDLE duplicate = duplicate_handle(handle);
 
-/*
- * Make MSVCRT's internal file descriptor control structure accessible
- * so that we can tweak OS handles and flags directly (we need MSVCRT
- * to treat our pipe handle as if it were a console).
- *
- * We assume that the ioinfo structure (exposed by MSVCRT.dll via
- * __pioinfo) starts with the OS handle and the flags. The exact size
- * varies between MSVCRT versions, so we try different sizes until
- * toggling the FDEV bit of _pioinfo(1)->osflags is reflected in
- * isatty(1).
- */
-typedef struct {
-	HANDLE osfhnd;
-	char osflags;
-} ioinfo;
-
-extern __declspec(dllimport) ioinfo *__pioinfo[];
-
-static size_t sizeof_ioinfo = 0;
+	/* Create a temp fd associated with the already open "new_handle". */
+	int new_fd = _open_osfhandle((intptr_t)new_handle, O_BINARY);
 
-#define IOINFO_L2E 5
-#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E)
+	assert((fd == 1) || (fd == 2));
 
-#define FPIPE 0x08
-#define FDEV  0x40
+	/*
+	 * Use stock dup2() to re-bind fd to the new handle.  Note that
+	 * this will implicitly close(1) and close both fd=1 and the
+	 * originally associated handle.  It will open a new fd=1 and
+	 * call DuplicateHandle() on the handle associated with new_fd.
+	 * It is because of this implicit close() that we created the
+	 * copy of the original.
+	 *
+	 * Note that the OS can recycle HANDLE (numbers) just like it
+	 * recycles fd (numbers), so we must update the cached value
+	 * of "console".  You can use GetFileType() to see that
+	 * handle and _get_osfhandle(fd) may have the same number
+	 * value, but they refer to different actual files now.
+	 *
+	 * Note that dup2() when given target := {0,1,2} will also
+	 * call SetStdHandle(), so we don't need to worry about that.
+	 */
+	dup2(new_fd, fd);
+	if (console == handle)
+		console = duplicate;
+	handle = INVALID_HANDLE_VALUE;
 
-static inline ioinfo* _pioinfo(int fd)
-{
-	return (ioinfo*)((char*)__pioinfo[fd >> IOINFO_L2E] +
-			(fd & (IOINFO_ARRAY_ELTS - 1)) * sizeof_ioinfo);
-}
+	/* Close the temp fd.  This explicitly closes "new_handle"
+	 * (because it has been associated with it).
+	 */
+	close(new_fd);
 
-static int init_sizeof_ioinfo(void)
-{
-	int istty, wastty;
-	/* don't init twice */
-	if (sizeof_ioinfo)
-		return sizeof_ioinfo >= 256;
-
-	sizeof_ioinfo = sizeof(ioinfo);
-	wastty = isatty(1);
-	while (sizeof_ioinfo < 256) {
-		/* toggle FDEV flag, check isatty, then toggle back */
-		_pioinfo(1)->osflags ^= FDEV;
-		istty = isatty(1);
-		_pioinfo(1)->osflags ^= FDEV;
-		/* return if we found the correct size */
-		if (istty != wastty)
-			return 0;
-		sizeof_ioinfo += sizeof(void*);
-	}
-	error("Tweaking file descriptors doesn't work with this MSVCRT.dll");
-	return 1;
-}
+	fd_is_interactive[fd] |= FD_SWAPPED;
 
-static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
-{
-	ioinfo *pioinfo;
-	HANDLE old_handle;
-
-	/* init ioinfo size if we haven't done so */
-	if (init_sizeof_ioinfo())
-		return INVALID_HANDLE_VALUE;
-
-	/* get ioinfo pointer and change the handles */
-	pioinfo = _pioinfo(fd);
-	old_handle = pioinfo->osfhnd;
-	pioinfo->osfhnd = new_handle;
-	return old_handle;
+	return duplicate;
 }
 
 #ifdef DETECT_MSYS_TTY
@@ -570,45 +550,25 @@ static void detect_msys_tty(int fd)
 			!wcsstr(name, L"-pty"))
 		return;
 
-	/* init ioinfo size if we haven't done so */
-	if (init_sizeof_ioinfo())
-		return;
-
-	/* set FDEV flag, reset FPIPE flag */
-	_pioinfo(fd)->osflags &= ~FPIPE;
-	_pioinfo(fd)->osflags |= FDEV;
+	fd_is_interactive[fd] |= FD_MSYS;
 }
 
 #endif
 
+/*
+ * Wrapper for isatty().  Most calls in the main git code
+ * call isatty(1 or 2) to see if the instance is interactive
+ * and should: be colored, show progress, paginate output.
+ * We lie and give results for what the descriptor WAS at
+ * startup (and ignore any pipe redirection we internally
+ * do).
+ */
+#undef isatty
 int winansi_isatty(int fd)
 {
-	int res = isatty(fd);
-
-	if (res) {
-		/*
-		 * Make sure that /dev/null is not fooling Git into believing
-		 * that we are connected to a terminal, as "_isatty() returns a
-		 * nonzero value if the descriptor is associated with a
-		 * character device."; for more information, see
-		 *
-		 * https://msdn.microsoft.com/en-us/library/f4s0ddew.aspx
-		 */
-		HANDLE handle = (HANDLE)_get_osfhandle(fd);
-		if (fd == STDIN_FILENO) {
-			DWORD dummy;
-
-			if (!GetConsoleMode(handle, &dummy))
-				res = 0;
-		} else if (fd == STDOUT_FILENO || fd == STDERR_FILENO) {
-			CONSOLE_SCREEN_BUFFER_INFO dummy;
-
-			if (!GetConsoleScreenBufferInfo(handle, &dummy))
-				res = 0;
-		}
-	}
-
-	return res;
+	if (fd >= 0 && fd <= 2)
+		return fd_is_interactive[fd] != 0;
+	return isatty(fd);
 }
 
 void winansi_init(void)
@@ -619,6 +579,10 @@ void winansi_init(void)
 	/* check if either stdout or stderr is a console output screen buffer */
 	con1 = is_console(1);
 	con2 = is_console(2);
+
+	/* Also compute console bit for fd 0 even though we don't need the result here. */
+	is_console(0);
+
 	if (!con1 && !con2) {
 #ifdef DETECT_MSYS_TTY
 		/* check if stdin / stdout / stderr are MSYS2 pty pipes */
@@ -662,12 +626,10 @@ void winansi_init(void)
  */
 HANDLE winansi_get_osfhandle(int fd)
 {
-	HANDLE hnd = (HANDLE) _get_osfhandle(fd);
-	if (isatty(fd) && GetFileType(hnd) == FILE_TYPE_PIPE) {
-		if (fd == 1 && hconsole1)
-			return hconsole1;
-		else if (fd == 2 && hconsole2)
-			return hconsole2;
-	}
-	return hnd;
+	if (fd == 1 && (fd_is_interactive[1] & FD_SWAPPED))
+		return hconsole1;
+	if (fd == 2 && (fd_is_interactive[2] & FD_SWAPPED))
+		return hconsole2;
+
+	return (HANDLE)_get_osfhandle(fd);
 }
-- 
2.11.0.rc3.windows.1

^ permalink raw reply related
* [PATCH v2 2/3] mingw: fix colourization on Cygwin pseudo terminals
From: Johannes Schindelin @ 2016-12-22 17:09 UTC (permalink / raw)
  To: git; +Cc: Alan Davies, Junio C Hamano, Pranit Bauva, Johannes Sixt,
	Beat Bolli
In-Reply-To: <cover.1482426497.git.johannes.schindelin@gmx.de>

From: Alan Davies <alan.n.davies@gmail.com>

Git only colours the output and uses pagination if isatty() returns 1.
MSYS2 and Cygwin emulate pseudo terminals via named pipes, meaning that
isatty() returns 0.

f7f90e0f4f (mingw: make isatty() recognize MSYS2's pseudo terminals
(/dev/pty*), 2016-04-27) fixed this for MSYS2 terminals, but not for
Cygwin.

The named pipes that Cygwin and MSYS2 use are very similar. MSYS2 PTY pipes
are called 'msys-*-pty*' and Cygwin uses 'cygwin-*-pty*'. This commit
modifies the existing check to allow both MSYS2 and Cygwin PTY pipes to be
identified as TTYs.

Note that pagination is still broken when running Git for Windows from
within Cygwin, as MSYS2's less.exe is spawned (and does not like to
interact with Cygwin's PTY).

This partially fixes https://github.com/git-for-windows/git/issues/267

Signed-off-by: Alan Davies <alan.n.davies@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/winansi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index 590d61cb1b..fa37695fca 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -562,8 +562,12 @@ static void detect_msys_tty(int fd)
 	name = nameinfo->Name.Buffer;
 	name[nameinfo->Name.Length] = 0;
 
-	/* check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX') */
-	if (!wcsstr(name, L"msys-") || !wcsstr(name, L"-pty"))
+	/*
+	 * Check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX')
+	 * or a cygwin pty pipe ('cygwin-XXXX-ptyN-XX')
+	 */
+	if ((!wcsstr(name, L"msys-") && !wcsstr(name, L"cygwin-")) ||
+			!wcsstr(name, L"-pty"))
 		return;
 
 	/* init ioinfo size if we haven't done so */
-- 
2.11.0.rc3.windows.1



^ permalink raw reply related
* [PATCH v2 0/3] Really fix the isatty() problem on Windows
From: Johannes Schindelin @ 2016-12-22 17:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Pranit Bauva, Johannes Sixt, Beat Bolli
In-Reply-To: <cover.1482342791.git.johannes.schindelin@gmx.de>

My previous fix may have fixed running the new
t/t6030-bisect-porcelain.sh script that tested the new bisect--helper,
which in turn used isatty() to determine whether it was running
interactively and was fooled by being redirected to /dev/null.

But it not only broke paging when running in a CMD window, due to
testing in the wrong worktree I also missed that it broke paging in Git
for Windows 2.x' Git Bash (i.e. a MinTTY terminal emulator).

Let's use this opportunity to actually clean up the entire isatty() mess
once and for all, as part of the problem was introduced by a clever hack
that messes with internals of the Microsoft C runtime, and which changed
recently, so it was not such a clever hack to begin with.

Happily, one of my colleagues had to address that latter problem
recently when he was tasked to make Git compile with Microsoft Visual C
(the rationale: debugging facilities of Visual Studio are really
outstanding, try them if you get a chance).

And incidentally, replacing the previous hack with the clean, new
solution, which specifies explicitly for the file descriptors 0, 1 and 2
whether we detected an MSYS2 pseudo-tty, whether we detected a real
Win32 Console, and whether we had to swap out a real Win32 Console for a
pipe to allow child processes to inherit it.

While at it (or, actually, more like: as I already made this part of v1
by mistake), upstream the patch carried in Git for Windows that supports
color when running Git for Windows in Cygwin terminals.

Changes since v1:

- rebased onto master

- unsquashed 2/3 which was improperly snuck in before,

- noted that Beat Bolli tested this (see
  https://github.com/git-for-windows/git/issues/997#issuecomment-268764693)

- fixed the confusing commit message by using Junio's suggested
  replacement

- added the missing white space between ">=" and "0"


Alan Davies (1):
  mingw: fix colourization on Cygwin pseudo terminals

Jeff Hostetler (1):
  mingw: replace isatty() hack

Johannes Schindelin (1):
  mingw: adjust is_console() to work with stdin

 compat/winansi.c | 198 +++++++++++++++++++++++--------------------------------
 1 file changed, 84 insertions(+), 114 deletions(-)


base-commit: 1d1bdafd64266e5ee3bd46c6965228f32e4022ea
Published-As: https://github.com/dscho/git/releases/tag/mingw-isatty-fixup-v2
Fetch-It-Via: git fetch https://github.com/dscho/git mingw-isatty-fixup-v2

Interdiff vs v1:

 diff --git a/compat/winansi.c b/compat/winansi.c
 index f51a2856d2..477209fce7 100644
 --- a/compat/winansi.c
 +++ b/compat/winansi.c
 @@ -108,7 +108,7 @@ static int is_console(int fd)
  	} else if (!GetConsoleScreenBufferInfo(hcon, &sbi))
  		return 0;
  
 -	if (fd >=0 && fd <= 2)
 +	if (fd >= 0 && fd <= 2)
  		fd_is_interactive[fd] |= FD_CONSOLE;
  
  	/* initialize attributes */
 @@ -555,7 +555,8 @@ static void detect_msys_tty(int fd)
  
  #endif
  
 -/* Wrapper for isatty().  Most calls in the main git code
 +/*
 + * Wrapper for isatty().  Most calls in the main git code
   * call isatty(1 or 2) to see if the instance is interactive
   * and should: be colored, show progress, paginate output.
   * We lie and give results for what the descriptor WAS at
 @@ -565,7 +566,7 @@ static void detect_msys_tty(int fd)
  #undef isatty
  int winansi_isatty(int fd)
  {
 -	if (fd >=0 && fd <= 2)
 +	if (fd >= 0 && fd <= 2)
  		return fd_is_interactive[fd] != 0;
  	return isatty(fd);
  }

-- 
2.11.0.rc3.windows.1


^ permalink raw reply
* Bug-Report: git-svn and backslash in SVN branch name
From: Michael Fladischer @ 2016-12-22 10:12 UTC (permalink / raw)
  To: git


[-- Attachment #1.1: Type: text/plain, Size: 1525 bytes --]

Hi,

I'm  trying to clone a SVN repo in order to migrate it to git but i hit
a wall with SVN branch names that contain a backslash, which seems to be
allowed in SVN but prohibited in git refs:

r289 = c4cb1f0c34e741a07de9673515c853d49c5522b9
(refs/remotes/origin/dicomBaseClass)
Found possible branch point: file:///path.repo/trunk/dicomBaseClass =>
file:///path.repo/tags/dicomBaseClass%5Cv0.1, 181
Initializing parent: refs/remotes/origin/tags/dicomBaseClass\v0.1@181
W: Ignoring error from SVN, path probably does not exist: (160013):
Filesystem has no item: File not found: revision 101, path
'/trunk/dicomBaseClass'
W: Do not be alarmed at the above message git-svn is just searching
aggressively for old history.
This may take a while on large repositories
Found possible branch point: file:///path.repo/dicomBaseClass =>
file:///path.repo/trunk/dicomBaseClass, 172
Initializing parent: refs/remotes/origin/tags/dicomBaseClass\v0.1@172
fatal: update_ref failed for ref
'refs/remotes/origin/tags/dicomBaseClass\v0.1@172': refusing to update
ref with bad name
'refs/remotes/origin/tags/dicomBaseClass\v0.1@172'
update-ref -m r75 refs/remotes/origin/tags/dicomBaseClass\v0.1@172
1fe0cc23e3cd56a1087562f8ca3d2e40cd2b30d4: command returned error: 128

The tag in case is "dicomBaseClass\v0.1@172".

Is there a way to mangle those names on the fly to get rid of the
backslashes or can they be handled in any other way to be compatible
with git?

Cheers,
-- 
Michael Fladischer
Fladi.at


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply
* Re: [PATCH] log: support 256 colors with --graph=256colors
From: Duy Nguyen @ 2016-12-22  9:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20161220165754.hkmnsxiwbcgn6uin@sigill.intra.peff.net>

On Tue, Dec 20, 2016 at 11:57 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Dec 20, 2016 at 07:39:29PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  I got mad after tracing two consecutive red history lines in `git log
>>  --graph --oneline` back to their merge points, far far away. Yeah
>>  probably should fire up tig, or gitk or something.
>>
>>  This may sound like a good thing to add, but I don't know how good it
>>  is compared to the good old 16 color palette, yet as I haven't tried it
>>  for long since it's just written.
>
> Hmm. At some point the colors become too close together to be easily
> distinguishable. In your code you have:
>
>> +     if (arg && !strcmp(arg, "256colors")) {
>> +             int i, start = 17, stop = 232;
>> +             column_colors_max = stop - start;
>> +             column_colors =
>> +                     xmalloc((column_colors_max + 1) * sizeof(*column_colors));
>> +             for (i = start; i < stop; i++) {
>> +                     struct strbuf sb = STRBUF_INIT;
>> +                     strbuf_addf(&sb, "\033[38;5;%dm", i);
>> +                     column_colors[i - start] = strbuf_detach(&sb, NULL);
>> +             }
>> +             column_colors[column_colors_max] = xstrdup(GIT_COLOR_RESET);
>> +             /* ignore the closet 16 colors on either side for the next line */
>> +             column_colors_step = 16;
>> +     }
>
> So you step by 16, over a set of 215 colors. That seems to give only 13
> colors, versus the original 16. :)
>
> I know that is a simplification. If you wrap around, then you get your
> 13 colors, and then another 13 colors that aren't _quite_ the same, and
> so on, until you've used all 256. I'm just not sure if the 1st and 14th
> color would be visually different enough for it to matter (I admit I
> didn't do any experiments, though).

Yep. If the jump sequence is a random one, we're less likely to run
into this. But I think Junio's "run git-log in 2 terminals with the
same coloring" convinces me randomization here is not the best thing.

The best solution would be select colors per text line, so we can pick
different colors. But I think that's a lot of computation (and
probably an NP problem too). The second best option is have a good,
predefined color palette. We don't need a red of all shades, we need
something that look distinct enough from the rest. I googled for this
first and failed. But I think I could approach it a different way:
collect colors that have names. That reduces the number of colors so
we can go back to "step 1 at a time" and still don't run into two
similar colors often.

>> ---graph::
>> +--graph[=<options>]::
>>       Draw a text-based graphical representation of the commit history
>>       on the left hand side of the output.  This may cause extra lines
>>       to be printed in between commits, in order for the graph history
>
> I wonder if we would ever want another use for "--graph=foo"

I do. See the screenshot in [1] from the original mail. I have to
stare at --graph so often lately that it might get my attention before
other things.

> I guess any such thing could fall under the name of "graph options", and we'd
> end up with "--graph=256colors,unicode" or something like that.

Exactly.

> I do suspect people would want a config option for this, though. I.e.,
> you'd want to enable it all the time if you have a terminal which can
> handle 256 colors, not just for a particular invocation.

Yeah. That also means we need the ability to override/negate config
options, perhaps something like --graph=-256colors.
-- 
Duy

^ permalink raw reply
* Re: [PATCH] log: support 256 colors with --graph=256colors
From: Duy Nguyen @ 2016-12-22  9:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqh95yldg4.fsf@gitster.mtv.corp.google.com>

On Wed, Dec 21, 2016 at 12:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> diff --git a/graph.c b/graph.c
>> index d4e8519..75375a1 100644
>> --- a/graph.c
>> +++ b/graph.c
>> @@ -78,6 +78,7 @@ static void graph_show_line_prefix(const struct diff_options *diffopt)
>>
>>  static const char **column_colors;
>>  static unsigned short column_colors_max;
>> +static int column_colors_step;
>>
>>  void graph_set_column_colors(const char **colors, unsigned short colors_max)
>>  {
>> @@ -234,10 +235,24 @@ void graph_setup_line_prefix(struct diff_options *diffopt)
>>  }
>>
>>
>> -struct git_graph *graph_init(struct rev_info *opt)
>> +struct git_graph *graph_init_with_options(struct rev_info *opt, const char *arg)
>>  {
>>       struct git_graph *graph = xmalloc(sizeof(struct git_graph));
>>
>> +     if (arg && !strcmp(arg, "256colors")) {
>> +             int i, start = 17, stop = 232;
>> +             column_colors_max = stop - start;
>> +             column_colors =
>> +                     xmalloc((column_colors_max + 1) * sizeof(*column_colors));
>> +             for (i = start; i < stop; i++) {
>> +                     struct strbuf sb = STRBUF_INIT;
>> +                     strbuf_addf(&sb, "\033[38;5;%dm", i);
>> +                     column_colors[i - start] = strbuf_detach(&sb, NULL);
>> +             }
>> +             column_colors[column_colors_max] = xstrdup(GIT_COLOR_RESET);
>> +             /* ignore the closet 16 colors on either side for the next line */
>> +             column_colors_step = 16;
>> +     }
>
> So you pre-fill a table of colors with 232-17=215 slots.  Is the
> idea that it is a co-prime with column_colors_step which is set to
> 16 so that going over the table with wraparound will cover all its
> elements?

Originally yes (because the next color would be more or less the same,
maybe brighter or darker a bit), then I went fancy with the rand()
thing...

>
>> @@ -382,6 +397,20 @@ static unsigned short graph_get_current_column_color(const struct git_graph *gra
>>   */
>>  static void graph_increment_column_color(struct git_graph *graph)
>>  {
>> +     if (column_colors_step) {
>> +             static int random_initialized;
>> +             int v;
>> +
>> +             if (!random_initialized) {
>> +                     srand((unsigned int)getpid());
>> +                     random_initialized = 1;
>> +             }
>> +             v = rand() % (column_colors_max - column_colors_step * 2);
>> +             graph->default_column_color += column_colors_step + v;
>> +             graph->default_column_color %= column_colors_max;
>> +             return;
>> +     }
>> +
>>       graph->default_column_color = (graph->default_column_color + 1) %
>>               column_colors_max;
>>  }
>
> This is too ugly to live as-is for two reasons.
>
>  - Do you really need rand()?  Doesn't this frustrate somebody who
>    runs the same "git log" in two terminals in order to view an
>    overly tall graph, expecting both commands that were started with
>    the same set of arguments to paint the same line in the same
>    color?

No we probably don't need rand(). The thinking was.. now that we have
a lot more colors to choose from, let's add some randomness, maybe
it'll reduce the chance of showing the same colors in the same line.

There was another concern with a fixed number of steps too, that we
could get into a stable jump sequence and never use all the colors
(e.g. step 3 with 6 colors total, to simplify). But I verify that
we'll use all the colors (at least until we allow people to customize
step and the number colors)
-- 
Duy

^ permalink raw reply
* Re: [PATCH] winansi_isatty(): fix when Git is used from CMD
From: Johannes Schindelin @ 2016-12-22  9:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, git, Pranit Bauva
In-Reply-To: <xmqqlgv9i04z.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Wed, 21 Dec 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > I prepared a patch series based on `pu`, on top of Hannes' patch, and
> > I also prepared a branch that is based on `master`, obviously without
> > Hannes' patch.
> 
> I think the latter is what you have at
> 
>  $ git fetch https://github.com/dscho/git mingw-isatty-fixup-master

Correct.

> If that is correct, I am inclined to fetch that and queue it on 'pu',
> replacing Hannes's patch, and then to ask him to Test/Ack it.

Okay, I will prepare v2 based on master and addressing your feedback.

Thanks,
Dscho

^ permalink raw reply
* Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references
From: Michael Haggerty @ 2016-12-22  8:15 UTC (permalink / raw)
  To: Marc Branchaud, Paul Mackerras; +Cc: git
In-Reply-To: <bfbd5992-da30-b1f0-59e5-a2f36d2e3062@xiplink.com>

On 12/21/2016 08:07 PM, Marc Branchaud wrote:
> On 2016-12-20 07:05 PM, Michael Haggerty wrote:
>> On 12/20/2016 04:01 PM, Marc Branchaud wrote:
>>> [...]
>>> Please don't change the remotebgcolor default.
>>>
>>> Also, perhaps the default remoterefbgcolor should be
>>>     set remoterefbgcolor $headbgcolor
>>> ?
>>>
>>> I say this because when I applied the series, without the last patch, I
>>> was miffed that the remote/ref colour had changed.
>>
>> This is a one-time inconvenience that gitk developers will experience. I
>> doubt that users jump backwards and forwards in gitk versions very often.
> 
> In what way do you mean it's restricted to gitk developers?

Maybe I misunderstood your objection.

While developing this, I realized that the very first time your run gitk
(i.e., when you don't already have a configuration file), it writes the
then-current default colors into your configuration file. If you later
switch gitk versions to a version with different default colors, the
colors from the first-run version are preserved (unless the names of the
variables used to hold the colors are changed).

So if you would run the tip version of my branch first, then the parent
of that version, you would continue to see the colors as modified by the
final commit. If you then switch to the master version, the remote
branch names go back to green (because it goes back to using
`headbgcolor` again), but the remote prefix stays light brown. I thought
you were unhappy about some form of this unexpected persistence problem.
But this problem will mostly be experienced by gitk developers who jump
back and forth between versions.

A normal user probably mostly moves forward through version numbers, and
only occasionally. Such a user, jumping from master to the tip of my
branch (assuming they haven't customized anything), would see

* local branches stay lime
* remote branch prefixes stay pale orange (they don't change to light
brown because the pale orange color from master is stored in their
configuration file)
* remote branch names change from lime to brown (because the
`remoterefbgcolor` configuration setting didn't exist before)

> Patch 12 introduces remoterefbgcolor, with a specific default value.
> Prior to that, the "ref part" of remote refs was rendered with
> headbgcolor.  Users who changed their headbgcolor are used to seeing the
> "ref part" of remote refs in the same color as their local heads.
> Applying patch 12 changes the "ref part" color of remote refs, for such
> users.
> 
> All I'm saying is that make the remoterefbgcolor default be $headbgcolor
> avoids this.

For somebody who thinks that most people will want local and
remote-tracking branch names to be rendered in the same color, your
suggestion would be an improvement.

But for somebody like me who thinks it is a good idea to render
remote-tracking branch names in a different color than local branch
names, this is a feature, not a bug. Even a user who explicitly
configured `headbgcolor` to, say, purple, wasn't necessarily expressing
a wish to have remote-tracking branch names rendered in purple. Until
now they had no choice to set these colors separately!

So even for somebody who configured this setting before, I think that
having the remote-tracking branch names change color when the user
upgrades to this version is a good thing, because it lets the user know
that these two things can now be colored independently. If they don't
like the new default brown color, such a user knows where to change it
(even to make it agree with `headbgcolor` if they want that).

But I understand that this is a matter of personal preference. I have
but one "vote" :-)

Michael


^ permalink raw reply
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox