* Re: [RFC/PATCH] add diffstat information to rebase
From: Stefan Beller @ 2016-12-22 22:13 UTC (permalink / raw)
To: git, Johannes.Schindelin; +Cc: Stefan Beller
In-Reply-To: <alpine.DEB.2.20.1612222239390.155951@virtualbox>
On Thu, Dec 22, 2016 at 1:41 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi Stefan,
>
> On Thu, 22 Dec 2016, Stefan Beller wrote:
>
>> 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(+)
>
> If it helps you, fine. But please make it opt-in. I would hate to see it
> in all my rebases, I find that information more confusing than helpful.
That's what I realized now that I played around with it for a day, because it actually
creates more lines than I can fit into my terminal in one screen now.
*Ideally* I would rather have a different formatting, e.g. maybe:
$ git checkout remotes/origin/js/sequencer-wo-die
$ git rebase -i --new-magic v2.10.0-rc2^
which produces:
pick d5cb9cbd64 Git 2.10-rc2 | Documentation/RelNo.., GIT-VERSION-GEN -..(+5, -1)
...
pick dbfad033d4 sequencer: do not die() in do_pick_commit() | sequencer.c - do_pick_commit (+7, -6)
pick 4ef3d8f033 sequencer: lib'ify write_message() | sequencer.c - write_message, do_pick_com..(+11, -9)
...
...
pick 88d5a271b0 sequencer: lib'ify save_opts() | sequencer.c - save_opts + sequencer_pick..(+20, -20)
pick 0e408fc347 sequencer: lib'ify fast_forward_to() | sequencer.c - fast_forward_to (+1, -1)
pick 55f5704da6 sequencer: lib'ify checkout_fast_forward() | sequencer.c - checkout_fast_forward (+6, -3)
pick 49fb937e9a sequencer: ensure to release the lock when w... | sequencer.c - read_and_refresh_cache (+6, -3)
When writing this example, I do notice that some sorts of commits put
nearly this exact information into the commit message. But that happens
when the commit is already well crafted and often it is refactoring.
A good example for the use case of this new format would be
origin/js/regexec-buf as there the commit message doesn't give
away what files and functions are touched.
Another very good example for the usefulness of the new format
appears to be origin/js/am-3-merge-recursive-direct, because:
* there are quite a few commits in the series.
(This feature seems to only be useful for long reshuffling series'
for interactive rebase in my mind.)
* It is not obvious by the commit title, which parts of the code
are touched. (files, functions)
* With a longer series, you can produce different valid orders for the
patches to be applied, e.g. compiling and testing works even if the patches
were applied in different orders.
Well actually the best use case are unfinished series,
with lots of "fixup" commits. :)
>
>> git-rebase--interactive.sh | 2 ++
>> 1 file changed, 2 insertions(+)
>
> Oh well. I guess I have to modify sequencer-i yet another time.
I think this feature can wait until the sequencer is in
and then we can build it on top, time permitting.
Thanks,
Stefan
^ permalink raw reply
* 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: Jacob Keller @ 2016-12-22 21:46 UTC (permalink / raw)
To: Stefan Monov; +Cc: Git mailing list
In-Reply-To: <CAJtFkWtjowyGaFfsCVd-HAZM2-3e0=CkkyYfxne8KRdYq5kJ9g@mail.gmail.com>
On Thu, Dec 22, 2016 at 1: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
Try setting "push.default" configuration setting, see "git help
config" and search for push.default
I *think* what you want is "push.default = current" but you should
read up on each option.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Jacob Keller @ 2016-12-22 21:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list, Norbert Kiesel
In-Reply-To: <xmqqinqbfz2r.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 22, 2016 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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>
>
That sounds familiar.
> 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 },
>
The unchanged context line should have its description re-worded to
something like "Use an experimental heuristic to improve diffs" as it
no longer uses only blank lines.
Everything else looked correct.
Thanks,
Jake
^ permalink raw reply
* 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: Junio C Hamano @ 2016-12-22 21:41 UTC (permalink / raw)
To: Stefan Monov; +Cc: git
In-Reply-To: <CAJtFkWtjowyGaFfsCVd-HAZM2-3e0=CkkyYfxne8KRdYq5kJ9g@mail.gmail.com>
Stefan Monov <logixoul@gmail.com> writes:
> 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>
Perhaps you are a target audience of
$ git config push.default current
then?
^ permalink raw reply
* Re: [RFC/PATCH] add diffstat information to rebase
From: Johannes Schindelin @ 2016-12-22 21:41 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20161222185609.21139-1-sbeller@google.com>
Hi Stefan,
On Thu, 22 Dec 2016, Stefan Beller wrote:
> 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(+)
If it helps you, fine. But please make it opt-in. I would hate to see it
in all my rebases, I find that information more confusing than helpful.
> git-rebase--interactive.sh | 2 ++
> 1 file changed, 2 insertions(+)
Oh well. I guess I have to modify sequencer-i yet another time.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 4/2] t5615-alternate-env: double-quotes in file names do not work on Windows
From: Johannes Schindelin @ 2016-12-22 21:38 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <54ba9cff-b6f7-7660-261f-393cd5181da6@kdbg.org>
Hi Hannes,
On Thu, 22 Dec 2016, Johannes Sixt wrote:
> 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.
Double-quotes are not allowed in file names in Windows. NTFS would allow
them just fine.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 3/3] mingw: replace isatty() hack
From: Johannes Schindelin @ 2016-12-22 21:37 UTC (permalink / raw)
To: Johannes Sixt
Cc: git, Jeff Hostetler, Junio C Hamano, Pranit Bauva, Beat Bolli
In-Reply-To: <e9f8a015-4106-fa88-082a-9e8c06ff61f3@kdbg.org>
Hi Hannes,
On Thu, 22 Dec 2016, Johannes Sixt wrote:
> 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?
You are correct, we must update `console` because `handle` is no longer
the handle we want.
The comment above only meant to reinforce that we have to forget about the
previous handle, too, as we might access something completely different
than a console otherwise.
Would you have a suggestion how to rephrase the comment to make it less
confusing?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 0/2] Really fix the isatty() problem on Windows
From: Johannes Schindelin @ 2016-12-22 21:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Junio C Hamano, Pranit Bauva
In-Reply-To: <ecc920bc-5323-a85c-c29f-9a3a7e4450e8@kdbg.org>
Hi Hannes,
On Thu, 22 Dec 2016, Johannes Sixt wrote:
> 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.
Thanks for confirming!
Dscho
^ permalink raw reply
* Re: [PATCH v1] git-p4: fix git-p4.pathEncoding for removed files
From: Junio C Hamano @ 2016-12-22 21:23 UTC (permalink / raw)
To: Luke Diamand; +Cc: Lars Schneider, Git Users
In-Reply-To: <CAE5ih7-=bD_ZoL5pFYfD2Qvy-XE24V_cgge0XoAvuoTK02EDfg@mail.gmail.com>
Luke Diamand <luke@diamand.org> writes:
> The change puts the logic into stripRepoPath() instead, which is
> indeed called from both of those functions (good), but also from
> splitFilesIntoBranches(), but only if self.useClientSpec is set. That
> function only gets used if we're doing the automatic branch detection
> logic, so it's possible that this code might now be broken and we
> wouldn't know.
>
> Lars, what do you think? Other than the above, the change looks good,
> so it may all be fine.
>
> (As an aside, this is the heart of the code that's going to need some
> careful rework if/when we ever move to Python3).
Thanks.
I'll merge this as-is to 'next', expecting that further refinement
can be done incrementally.
^ permalink raw reply
* 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox