Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
On Thu, Dec 22, 2016 at 01:12:12PM -0800, Junio C Hamano wrote:
> Jacob Keller 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.d74n6lhgp2hex...@sigill.intra.peff.net>
FWIW, I was swayed in the other direction by later messages in the
thread. Especially your noting that the "compaction" name has always
been labeled experimental, and Michael's argument in:
http://public-inbox.org/git/8dbbd28b-af60-5e66-ae27-d7cddca23...@alum.mit.edu/
I.e., we could keep calling it "--indent-heuristic", and probably drop
the other heuristic entirely as a failed experiment.
I can live with it either way, but since I am being quoted as the source
of the suggestion, I feel like that's an invitation to add my 2 cents. :)
Liberal quoting below since I am adding Michael to the cc list.
-Peff
> 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
> Signed-off-by: Junio C Hamano
> ---
> 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_fi
Re: [PATCH] git-svn: escape backslashes in refnames
On 2016-12-23 02:42, Eric Wong wrote: > Hi Michael, the patch below should fix things up. Thank you Eric, I was able to successfully fetch the SVN tag with your patch applied. Cheers, -- Michael Fladischer Fladi.at signature.asc Description: OpenPGP digital signature
[PATCH] git-svn: escape backslashes in refnames
Hi Michael, the patch below should fix things up.
Junio: this should go to 'maint', pull request below.
8<---
Subject: [PATCH] git-svn: escape backslashes in refnames
This brings git-svn refname escaping up-to-date with
commit a4c2e69936df8dd0b071b85664c6cc6a4870dd84
("Disallow '\' in ref names") from May 2009.
Reported-by: Michael Fladischer
Message-ID:
Signed-off-by: Eric Wong
---
The following changes since commit a274e0a036ea886a31f8b216564ab1b4a3142f6c:
Sync with maint-2.10 (2016-12-05 11:25:47 -0800)
are available in the git repository at:
git://bogomips.org/git-svn.git svn-escape-backslash
for you to fetch changes up to 22af6fef9b6538c9e87e147a920be9509acf1ddd:
git-svn: escape backslashes in refnames (2016-12-23 01:37:36 +)
Eric Wong (1):
git-svn: escape backslashes in refnames
perl/Git/SVN.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 711d2687a3..98518f4ddb 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -490,7 +490,7 @@ sub refname {
#
# Additionally, % must be escaped because it is used for escaping
# and we want our escaped refname to be reversible
- $refname =~ s{([ \%~\^:\?\*\[\t])}{sprintf('%%%02X',ord($1))}eg;
+ $refname =~ s{([ \%~\^:\?\*\[\t\\])}{sprintf('%%%02X',ord($1))}eg;
# no slash-separated component can begin with a dot .
# /.* becomes /%2E*
--
EW
Re: Feature request: git rebase --no-edit --continue
Uff, I forgot about the CC of my last reply. Why not git commit -C HEAD && git rebase --continue ? Ciao, Johannes Thanks this is a cleaner solution. I guess because I was too fixed upon knowing of the existence of the no-edit option from other git commands that I forget about this, even though I am using it quite often.
Re: [PATCH v2 1/3] mingw: adjust is_console() to work with stdin
Beat Bolli writes:
>> @@ -84,6 +84,7 @@ static void warn_if_raster_font(void)
>> static int is_console(int fd)
>> {
>> CONSOLE_SCREEN_BUFFER_INFO sbi;
>> +DWORD mode;
>
> Nit: can we move this definition into the block below where it's used?
>
>> 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) {
>
> Right here:
> + DWORD mode;
>
>> +if (!GetConsoleMode(hcon, &mode))
>> +return 0;
>> +} else if (!GetConsoleScreenBufferInfo(hcon, &sbi))
>> return 0;
As these patches have been already merged to 'next' a few hours ago
as part of today's integration cycle, please make any further
refinement (if necessary) as incremental updates.
Thanks.
Re: [PATCH v2 3/3] mingw: replace isatty() hack
Hi Hannes, On Thu, 22 Dec 2016, Johannes Sixt wrote: > Am 22.12.2016 um 22:37 schrieb Johannes Schindelin: > > > Would you have a suggestion how to rephrase the comment to make it > > less confusing? > > Perhaps > >* This might close the cached console handle. >* We must cache the live duplicate instead. > > Then update the cache before the dup2, because at this time all 3 of console, > handle, and duplicate are live and cannot be recycled: > > if (console == handle) > console = duplicate; > dup2(new_fd, fd); Good point. I reworded the comment slightly differently (see the interdiff in v3 0/3), hope you don't mind. Ciao, Dscho
[PATCH v3 3/3] mingw: replace isatty() hack
From: Jeff Hostetler
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
Tested-by: Beat Bolli
Signed-off-by: Johannes Schindelin
---
compat/winansi.c | 181 +--
1 file changed, 70 insertions(+), 111 deletions(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index fa37695fca..56658ec4f8 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -6,9 +6,12 @@
#include "../git-compat-util.h"
#include
#include
+#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_MSYS0x4
/*
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,47 @@ static HANDLE duplicate_handle(HANDLE hnd)
return hresult;
}
-
-/*
- * 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;
-
-#define IOINFO_L2E 5
-#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E)
-
-#define FPIPE 0x08
-#define FDEV 0x40
-
-static inline ioinfo* _pioinfo(int fd)
-{
- return (ioinfo*)((char*)__pioinfo[fd >> IOINFO_L2E] +
- (fd & (IOINFO_ARRAY_ELTS - 1)) * sizeof_ioinfo);
-}
-
-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;
-}
-
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;
+ /*
+* 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 we need to update the cached console handle to the
+* duplicated one because the dup2() call will implicitly close
+* the original one.
+*
+* Note that dup2() when given target := {0,1,2} will also
+* call SetStdHandle(), so we don't need to worry about that.
+*/
+ if (console == handle)
+ console = duplicate;
+ dup2(new_fd, fd);
+
+ /* Close the temp fd. This explicitly cl
[PATCH v3 1/3] mingw: adjust is_console() to work with stdin
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
---
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
[PATCH v3 2/3] mingw: fix colourization on Cygwin pseudo terminals
From: Alan Davies
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
Signed-off-by: Johannes Schindelin
---
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--ptyN-XX') */
- if (!wcsstr(name, L"msys-") || !wcsstr(name, L"-pty"))
+ /*
+* Check if this could be a MSYS2 pty pipe ('msys--ptyN-XX')
+* or a cygwin pty pipe ('cygwin--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
[PATCH v3 0/3] Really fix the isatty() problem on Windows
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 v2:
- reworded the comment about "recycling handles"
- moved the reassignment of the `console` variable before the dup2()
call so that it is valid at all times
- removed the "handle = INVALID_HANDLE_VALUE" assignment, as the local
variable `handle` is not used afterwards anyway
- generated the patches with --patience in the hope to improve
readability
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 | 195 +++
1 file changed, 81 insertions(+), 114 deletions(-)
base-commit: 1d1bdafd64266e5ee3bd46c6965228f32e4022ea
Published-As: https://github.com/dscho/git/releases/tag/mingw-isatty-fixup-v3
Fetch-It-Via: git fetch https://github.com/dscho/git mingw-isatty-fixup-v3
Interdiff vs v2:
diff --git a/compat/winansi.c b/compat/winansi.c
index 477209fce7..56658ec4f8 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -494,19 +494,16 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
* 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 we need to update the cached console handle to the
+ * duplicated one because the dup2() call will implicitly close
+ * the original one.
*
* 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;
+ dup2(new_fd, fd);
/* Close the temp fd. This explicitly closes "new_handle"
* (because it has been associated with it).
--
2.11.0.rc3.windows.1
Re: Feature request: git rebase --no-edit --continue
Hi Daniel, On Thu, 22 Dec 2016, Daniel Chumak wrote: > Is there a reason why there is no '--no-edit' option for git rebase? I would > like to use this option after editing a commit and continuing the current > interactive rebase without having to change the commit message. Why not git commit -C HEAD && git rebase --continue ? Ciao, Johannes
Re: [RFC/PATCH] add diffstat information to rebase
Hi Stefan, On Thu, 22 Dec 2016, Stefan Beller wrote: > *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) > ... That sounds more like a patch adding a new placeholder to the user formats than a patch modifying rebase -i; by using rebase.instructionFormat you can implement that "new magic", and the new functionality may even benefit more obscure use cases. Ciao, Dscho
Re: [PATCH v2 1/3] mingw: adjust is_console() to work with stdin
On 22.12.16 18:08, Johannes Schindelin wrote:
> 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
> ---
> 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;
Nit: can we move this definition into the block below where it's used?
> 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) {
Right here:
+ DWORD mode;
> + if (!GetConsoleMode(hcon, &mode))
> + return 0;
> + } else if (!GetConsoleScreenBufferInfo(hcon, &sbi))
> return 0;
>
> /* initialize attributes */
>
Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
Jacob Keller writes:
>> { 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.
Thanks. The final copy I pushed out has that change.
Re: Feature request: git rebase --no-edit --continue
On Thu, Dec 22, 2016 at 2:35 PM, Daniel Chumak wrote: > Is there a reason why there is no '--no-edit' option for git rebase? Nobody added it so far. > I would > like to use this option after editing a commit and continuing the current > interactive rebase without having to change the commit message. > You're welcome to write a patch for it. :) On the other hand you could just use EDITOR=true git rebase --continue
Re: [RFC/PATCH] add diffstat information to rebase
Stefan Beller writes: > $ 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 an interesting and thought-provoking idea ;-). In practice, you would probably be touching the same file over and over again in the series you are rebasing, when you are doing "many miniscule commits recording experiments and dead ends, with an intention to clean it up later", and by definition, your subject lines are useless series of "oops", "fix", etc. The subject and list of filenames would probably not make a good "summary" of the changes for each commit. Stepping back a bit, right now, when the user asks "git commit" to supply material to help writing a good commit message, we punt on mechanically generating a good summary and instead just show output of "diff --cached". If we can come up with a way to mechanically generate a concise summary for the purpose of annotating "rebase -i" instruction, we probably can reuse that and append it at the end of the log editor "git commit" spawns when it is run without "-v". Also, this makes me wonder if the ideal endgame might be to depend on the current "rebase -i" UI as little as possible. "rebase -i" is "interactive" only to the extent that you can interact in your text editor the order and the fashion in which the changes are applied. If we instead teach either gitk or tig to easily allow you to "tick" each commit you see in their UI and generate the instruction used by the sequencer, and feed that and actually drive the sequencer to execute it (perhaps inside a temporary/throwaway working tree) while you are still in gitk or tig and reload the UI dynamically to let you view the result, the overall user experience would become a lot more "interactive".
Feature request: git rebase --no-edit --continue
Is there a reason why there is no '--no-edit' option for git rebase? I would like to use this option after editing a commit and continuing the current interactive rebase without having to change the commit message.
Re: [PATCH v2 3/3] mingw: replace isatty() hack
Am 22.12.2016 um 22:37 schrieb Johannes Schindelin:
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.
It is like accessing a pointer after free().
When I read the paragraph for the first time, I expected some
information to be saved, then some handles to be closed and re-opened,
which would possibly recycle a handle value, and then same state to be
resurrected depending on the information saved earlier.
But nothing of this kind happens, only:
dup2(new_fd, fd);
if (console == handle)
console = duplicate;
which is necessary and, once one has understood the context, obvious.
Would you have a suggestion how to rephrase the comment to make it less
confusing?
Perhaps
* This might close the cached console handle.
* We must cache the live duplicate instead.
Then update the cache before the dup2, because at this time all 3 of
console, handle, and duplicate are live and cannot be recycled:
if (console == handle)
console = duplicate;
dup2(new_fd, fd);
-- Hannes
Re: [PATCH 4/2] t5615-alternate-env: double-quotes in file names do not work on Windows
On Thu, Dec 22, 2016 at 08:06:02PM +0100, 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. Thank you for double-checking (and sorry if my initial question was confusing; I somehow missed the mention of dq in your subject line, and just read the message body). Your patch seems like the best solution. -Peff
What's cooking in git.git (Dec 2016, #07; Thu, 22)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. Even though I try not to do two "What's cooking" report back to back, I wanted to push out a few topics that we want to have in 'master' soonish on 'next' before things really quiet and slow down due to year-end holidays. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html Here are the summaries: Will merge to 'master'. + jc/push-default-explicit 10-31/11-01 #2 + sb/submodule-config-cleanup 11-22/11-23 #3 + va/i18n-perl-scripts 12-14/12-19 #16 + cp/merge-continue12-15/12-19 #4 + bw/transport-protocol-policy 12-15/12-19 #6 + ls/filter-process12-18/12-19 #2 + ld/p4-compare-dir-vs-symlink 12-18/12-20 #1 + jk/difftool-in-subdir12-11/12-21 #4 + sb/submodule-embed-gitdir12-12/12-21 #6 + gv/p4-multi-path-commit-fix 12-19/12-21 #1 + mk/mingw-winansi-ttyname-termination-fix 12-20/12-21 #1 + lt/shortlog-by-committer 12-20/12-21 #3 + va/i18n-even-more12-20/12-22 #1 + ls/p4-lfs12-20/12-22 #1 + js/mingw-isatty 12-22/12-22 #3 + bw/realpath-wo-chdir 12-22/12-22 #5 + bw/grep-recurse-submodules 12-22/12-22 #7 Will merge to 'next'. - jc/git-open-cloexec 11-02 #3 - ls/p4-path-encoding 12-18 #1 - mh/fast-import-notes-fix-new 12-20 #1 - jc/abbrev-autoscale-config 12-22 #1 - jc/retire-compaction-heuristics 12-22 #1 - nd/config-misc-fixes 12-22 #3 -- [Stalled] * jk/nofollow-attr-ignore (2016-11-02) 5 commits - exclude: do not respect symlinks for in-tree .gitignore - attr: do not respect symlinks for in-tree .gitattributes - exclude: convert "check_index" into a flags field - attr: convert "macro_ok" into a flags field - add open_nofollow() helper As we do not follow symbolic links when reading control files like .gitignore and .gitattributes from the index, match the behaviour and not follow symbolic links when reading them from the working tree. This also tightens security a bit by not leaking contents of an unrelated file in the error messages when it is pointed at by one of these files that is a symbolic link. Perhaps we want to cover .gitmodules too with the same mechanism? * nd/worktree-move (2016-11-28) 11 commits . worktree remove: new command . worktree move: refuse to move worktrees with submodules . worktree move: accept destination as directory . worktree move: new command . worktree.c: add update_worktree_location() . worktree.c: add validate_worktree() . copy.c: convert copy_file() to copy_dir_recursively() . copy.c: style fix . copy.c: convert bb_(p)error_msg to error(_errno) . copy.c: delete unused code in copy_file() . copy.c: import copy_file() from busybox "git worktree" learned move and remove subcommands. Reported to break builds on Windows. * jc/bundle (2016-03-03) 6 commits - index-pack: --clone-bundle option - Merge branch 'jc/index-pack' into jc/bundle - bundle v3: the beginning - bundle: keep a copy of bundle file name in the in-core bundle header - bundle: plug resource leak - bundle doc: 'verify' is not about verifying the bundle The beginning of "split bundle", which could be one of the ingredients to allow "git clone" traffic off of the core server network to CDN. While I think it would make it easier for people to experiment and build on if the topic is merged to 'next', I am at the same time a bit reluctant to merge an unproven new topic that introduces a new file format, which we may end up having to support til the end of time. It is likely that to support a "prime clone from CDN", it would need a lot more than just "these are the heads and the pack data is over there", so this may not be sufficient. Will discard. * mh/connect (
Re: [RFC/PATCH] add diffstat information to rebase
On Thu, Dec 22, 2016 at 1:41 PM, Johannes Schindelin 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
Re: Making it possible to do “git push origin” instead of “git push origin ”, without having to one-time prepare each branch for it
On Thu, Dec 22, 2016 at 1:14 PM, Stefan Monov 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 > > At [1] and [2] I've seen that if I do this once: > > git push -u origin > > 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
Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
On Thu, Dec 22, 2016 at 1:12 PM, Junio C Hamano wrote:
> Jacob Keller 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.d74n6lhgp2hex...@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
> Signed-off-by: Junio C Hamano
> ---
> 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
Re: Making it possible to do “git push origin” instead of “git push origin ”, without having to one-time prepare each branch for it
Stefan Monov 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 Perhaps you are a target audience of $ git config push.default current then?
Re: [RFC/PATCH] add diffstat information to rebase
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
Re: [PATCH 4/2] t5615-alternate-env: double-quotes in file names do not work on Windows
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
Re: [PATCH v2 3/3] mingw: replace isatty() hack
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
Re: [PATCH 0/2] Really fix the isatty() problem on Windows
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
Re: [PATCH v1] git-p4: fix git-p4.pathEncoding for removed files
Luke Diamand 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.
Re: Making it possible to do “git push origin” instead of “git push origin ”, without having to one-time prepare each branch for it
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 ` first? On Thu, Dec 22, 2016 at 11:14 PM, Stefan Monov 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 > > At [1] and [2] I've seen that if I do this once: > > git push -u origin > > 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
Making it possible to do “git push origin” instead of “git push origin ”, without having to one-time prepare each branch for it
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 At [1] and [2] I've seen that if I do this once: git push -u origin 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
Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
Jacob Keller 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.d74n6lhgp2hex...@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
Signed-off-by: Junio C Hamano
---
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.indentheuris
Re: [PATCH v2 3/3] mingw: replace isatty() hack
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;
}
Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
Brandon Williams 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.
Hello
Hello, Complement of the Season, I'M Anessa female 25 years old single, I am contacting you because I will be relocating to your country. I will send you my photos and also tell you much about my self. Thanks. Sincerely yours Anessa.
Re: [PATCH] log: support 256 colors with --graph=256colors
Duy Nguyen 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:5f) 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.
Re: [PATCH 4/2] t5615-alternate-env: double-quotes in file names do not work on Windows
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
[RFC/PATCH] add diffstat information to rebase
Signed-off-by: Stefan Beller
---
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
Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
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
Re: [PATCH 0/2] Really fix the isatty() problem on Windows
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
Re: [PATCH v2 0/3] Really fix the isatty() problem on Windows
Johannes Schindelin 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.
Re: [PATCH v2 0/3] Really fix the isatty() problem on Windows
Junio C Hamano writes: > Johannes Schindelin 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 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 X. 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.
Re: [PATCH v2 0/3] Really fix the isatty() problem on Windows
Hi Junio, On Thu, 22 Dec 2016, Junio C Hamano wrote: > Johannes Schindelin 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.
Re: [PATCH] mailinfo.c: move side-effects outside of assert
Jeff King 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.
Re: [PATCH v2 0/3] Really fix the isatty() problem on Windows
Johannes Schindelin 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.
Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
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
[PATCH v2 2/3] mingw: fix colourization on Cygwin pseudo terminals
From: Alan Davies
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
Signed-off-by: Johannes Schindelin
---
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--ptyN-XX') */
- if (!wcsstr(name, L"msys-") || !wcsstr(name, L"-pty"))
+ /*
+* Check if this could be a MSYS2 pty pipe ('msys--ptyN-XX')
+* or a cygwin pty pipe ('cygwin--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
[PATCH v2 3/3] mingw: replace isatty() hack
From: Jeff Hostetler
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
Tested-by: Beat Bolli
Signed-off-by: Johannes Schindelin
---
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
#include
+#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_MSYS0x4
/*
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, HA
[PATCH v2 1/3] mingw: adjust is_console() to work with stdin
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
---
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
[PATCH v2 0/3] Really fix the isatty() problem on Windows
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
Bug-Report: git-svn and backslash in SVN branch name
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 signature.asc Description: OpenPGP digital signature
Re: [PATCH] log: support 256 colors with --graph=256colors
On Tue, Dec 20, 2016 at 11:57 PM, Jeff King 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
>> ---
>> 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[=]::
>> 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
Re: [PATCH] log: support 256 colors with --graph=256colors
On Wed, Dec 21, 2016 at 12:26 AM, Junio C Hamano wrote:
> Nguyễn Thái Ngọc Duy 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
Re: [PATCH] winansi_isatty(): fix when Git is used from CMD
Hi Junio, On Wed, 21 Dec 2016, Junio C Hamano wrote: > Johannes Schindelin 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
Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references
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
