Gitk f5 filenametoolong

[max630]

This should fix #1987

[dscho]

That's a pretty good start (and don't worry about the failed checks, that's a Homebrew bug for which I opened Homebrew/brew#5666).

In the current version, I see a few limitations, still, that I think we can address before merging:

• It adds a hard-coded, Windows-specific command-line limit. That sounds like it leaves trouble for other platforms (which also have command-line length limites).
• It simply swallows the negative refs after the command-line length was reached. Which means that the command-line is not only within limits, but also incorrect.

Now, the direction I would like this PR to follow is to move from git log to git rev-list because that latter command would be the appropriate low-level command to use, anyway, and it allows us to use --stdin to provide the refs via a pipe without limit.

Of course, a brief test revealed that the --pretty=raw output of boundary commits seems to differ between rev-list and log: the rev-list (correctly) prefixes the first commit hash on the commit line with the boundary marker -, while log writes the boundary marker followed by a space followed by the first commit hash. Also, rev-list adds an empty line to the output.

However, I think that this should be reasonably straight-forward to accommodate. And it would be the better fix for the issue, extending to all platforms supported by Git.

[max630]

• I thought it could work in line with f2c696f
• these negavite patterns are there to limit work for git log. It some are missing it would just fetch some already known commits. I believe, it should not make a big harm except maybe performance degradation. I have not verified it though.

I'll try to implement it as you described.

[PhilipOakley]

minor punctuation suggestions if updated:

• perhaps add a comma ("them, the") to match the when clause to make comprehension simpler.
• Capitalise "Fix" in second paragraph.

[dscho]

* I thought it could work in line with [f2c696f](https://github.com/git-for-windows/git/commit/f2c696f706d5a19c558f0479d1db7efc0ef42aac)

Right. I thought that that patch would come back to haunt us. It is totally not upstreamable, not only because it uses that hard-coded approach that I don't think would be accepted by Paul Mackerras (who is still the gitk maintainer AFAICT), but also because of the intentionally fake author.

* these negavite patterns are there to limit work for `git log`. It some are missing it would just fetch some already known commits. I believe, it should not make a big harm except maybe performance degradation. I have not verified it though.

Good point, but I think that we would want the --stdin method in general because it might not be purely a performance issue in other, similar cases. And if we can come up with a general fix, I'd much prefer that one.

I'll try to implement it as you described.

Thank you!

[max630]

and it allows us to use --stdin to provide the refs via a pipe without limit

apparently git log also supports --stdin (link), would it be OK to still use it?

[max630]

I'm struggling to find a way to make 2-way redirection in tcl. It looks like combination of chan and exec could work, but manual for exec says that redirectiondo not work in Tk/Windows. Maybe it's outdated (why wouldn't it work?), needs some protoing.

another way would be to use a real temporary file but it feels like a drawback.

[mfriedrich74]

Redirection works in Linux and in Windows, but exec is very limited regarding this. You could try pipes using open "|...".

If you refer to:

Note: stdout and stderr are not available in Windows when Tcl is invoked as wish, because Windows does not support stdio for GUI apps.

This is wrong. It is supported, but not visible until a console window is attached to the process. The process can do that using AttachConsole. Or you find a way to redirect the output to a text window.

[max630]

open with "|.." is what is used now, but now I need to redirect both stdin and stdout

If you refer to:

it was rather this:

The Tk console text widget does not provide real standard IO capabilities. Under Tk, when redirecting from standard input, all applications will see an immediate end-of-file; information redirected to standard output or standard error will be discarded.

I tried this example, it seems working. Though maybe I should have tried also starting it as wish. Will see.

[lostindark]

I tried this fix locally and it doesn't fix the error I saw. Let me know if you need any logs.

[dscho]

Let me know if you need any logs.

I don't think that it will work if you put the burden of driving your issue forward on others...

[dscho]

/AzurePipelines run git-for-windows.git

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[max630]

It looks like there is a problem. Apparently git log --stdin does not support passing --not, it fails with message "fatal: options not supported in --stdin mode". And it is the hashes after "--not" which was causing the issue. Trying to pass "--not" in command line does not seem to help as well, the commits are treated as positive selection.

[max630]

well maybe the subsequent arguments may prepended with ^

[max630]

@dscho it is rewritten now to use stdin as you have requested

[max630]

Fixed syntax error

[dscho]

It looks like there is a problem. Apparently git log --stdin does not support passing --not, it fails with message "fatal: options not supported in --stdin mode". And it is the hashes after "--not" which was causing the issue. Trying to pass "--not" in command line does not seem to help as well, the commits are treated as positive selection.

Hmm. I tried running this:

set input "HEAD^\n--not\nHEAD^^^"
set fd [open [concat | git log -s --stdin <<$input]]
set data [read $fd]
puts "data: $data"
if {[catch {close $fd} err]} {
	puts "ls command failed: $err"
}

... and it gave me the expected output: the first two of the latest three commits...

[dscho]

... and it gave me the expected output: the first two of the latest three commits...

Oh never mind, the <<$input is concatenated, so the whitespace means that only the first component of $input is used as stdin...

[dscho]

the <<$input is concatenated, so the whitespace means that only the first component of $input is used as stdin...

Hah! If I construct input by appending values with the \\n separator, it is used as stdin in its entirety, and those \\n are translated into newlines, as desired.

Together with replacing the --not flag by the ^ prefix, I think we have a way forward here.

[dscho]

@max630 thank you for your hard work on this PR. The issue would not have been fixed (in #2170) without you.