* Re: Proposal for an increased `gitk` cohesion with `git stash`.
2016-12-12 9:36 ` Proposal for an increased `gitk` cohesion with `git stash` Paul Mackerras
@ 2016-12-14 0:36 ` Uxío Prego
0 siblings, 0 replies; 2+ messages in thread
From: Uxío Prego @ 2016-12-14 0:36 UTC (permalink / raw)
To: git, Paul Mackerras
Dearest all,
am sorry my previous message did not enter the list (cross my fingers this will). I won't be pasting it verbatim because shame on me it leaked zombie processes (but that part got silently dropped out by kind Paul).
In case anyone could be interested in the topic, and because a thorough reply will take me some time, my most recent edit of this is hosted at https://gist.githubusercontent.com/uprego/d8c3c059c56ebb911974bb905157a81e/raw/6a08d9e0ce9c2b1decd4ed92acc924961c7f7769/gitk%2520multi%2520stash%2520patch. All problems shown I still think is a nice start (of course p.o.c / pre alpha) if anyone ever wanted to get this working or even fix the current problems it has.
As Paul recommend I'll be reworking and giving a patch against a rev of his upstream.
I'm going to try his code tips to improve non obvious design choices, and (even he doesn’t commented it and seems to me most important) really put an extra effort in not changing the behaviour of `gitk` (i.e. started without '--all').
Then some testing against large repos, github.com/cartodb/cartodb then github.com/odoo/oodo finally Linux; will be done.
The performance issue Paul points to, I don't think is impacting me, but now I reckon (just as one example) there are people who develop using IDEs that leave garbaged unuseful stashes, and that has to be taken into account as scenario. And the large repos. But this file event handlers thing is something that will make me lag to fix it, even surprised me because the remaining of the subroutines that I patched are just doing the same I typed, I just replicated near source (general revs processing) because I have no idea Tcl, even do not give a shit, but have to say Tcl is fun C: and an interesting discovery though. I hope that was not a trick to get me into improving the performance of the near loop that process ALL involved revs (and the similar for refs)! I'm old and tired to get into performance hacking.
I guess you know, the underworked grep must be an easy solve, probably excluding ' refs/stash' because a branch named 'refs/stash' is allowed but not a branch named ' refs/stash' (IDK which version I was trying but I will try both 1.x.y and 2.x.y in time).
Finally... if you don't leverage stashing too much, what is the practice? committing ephemeral to later reset and recommit? I hope I don't just needed a lesson on git-reset instead of all this.
Please pardon my potentially mangler mail client. Yours sincerely, regards and thanks for your time,
> On 12 Dec 2016, at 10:36, Paul Mackerras <paulus@ozlabs.org> wrote:
>
> Hi Uxio,
>
> On Thu, Sep 08, 2016 at 03:41:29PM +0200, Uxío Prego wrote:
>> Hello, please forgive me for not introducing me.
>>
>> +-----------+
>> |Description|
>> +-----------+
>>
>> Patch for showing all stashes in `gitk`.
>>
>> +-----------+
>> |The problem|
>> +-----------+
>>
>> Being `gitk` one of the best tools for navigating and understanding graphs
>> of git repo revs, I got used to have it for home use, some years ago, soon
>> for office use too.
>>
>> At some point I got used to invoke always `gitk --all` in order to keep
>> tracking of tags when using the program for building, when possible, stable
>> versions of any GNU/Linux software I would want to use.
>>
>> It seems `gitk --all` uses `git show-ref` for showing remotes information.
>> A side effect of this is that the most recent stash gets shown in `gitk
>> --all`. After learning what the stash was, I got used to it.
>>
>> Later, when at jobs, I got used to have a stash on all working branch tips.
>> So I often would have several stashes in `git stash list` but only the last
>> one in `gitk --all`. That annoyed me for about a year, finally I got
>> working in `gitk` to show a stash status more similar to what `git stash
>> list` shows.
>>
>> +-----------+
>> |The feature|
>> +-----------+
>>
>> Show all stashes in `gitk` instead of only the last one.
>
> This seems like a good feature, although I don't use stashes myself.
>
>> +------------------+
>> |Why it's desirable|
>> +------------------+
>>
>> In order to have better visual control when working on repos that have
>> several active branches and there are needed quick context changes between
>> them with the features that `git stash [apply [STASHNAME]| list | pop
>> [STASHNAME]| push | save | show [[-p] STASHNAME]]`.
>>
>> In order to have a better cohesion between the mentioned features and `gitk
>> --all`.
>>
>> +------------------------+
>> |Caveats and side effects|
>> +------------------------+
>>
>> With this patch several side effects happen:
>>
>> * Stashes are shown even invoking `gitk`, not only when running `gitk
>> --all`. If this is a problem, I can keep working in the patch to avoid this.
>>
>> * The most recent stash, which was previously shown as 'stash' because its
>> corresponding `git show-ref` output line reads 'refs/stash', gets shown as
>> 'stash@{0}'. Not removing lines with 'stash' when calling `git show-ref`
>> gets it shown both as 'stash' as usual and as 'stash@{0}'.
>>
>> +--------------------------+
>> |Non-obvious design choices|
>> +--------------------------+
>>
>> There are many improvable things consequence of never having edited
>> anything Tcl before this. Besides, its working for me as a proof of
>> concept, both in Debian 7 Wheezy and 8 Jessie.
>>
>> The origin document of the following diff is `gitk` as it ships in Debian 8
>> Jessie. I have not tried yet but if required I would be willing to rework
>> it for the repo master.
>
> A patch against the latest version in my git repo at
> git://ozlabs.org/~paulus/gitk would be better.
>
>> `gitk-stash-list-ids.sh` is a shell script that performs `git stash list
>> --pretty=format:"%H"`, i.e. show rev hashes for all stashes in the fashion
>> that `git rev-list --all` does its default output. I did this because I
>> could not work out a better pure Tcl solution.
>
> Hmmm, I don't want gitk to have to depend on an external script.
> It should be possible to make Tcl execute the git command directly,
> though (see below).
>
>> +--------------------+
>> |Unified diff follows|
>> +--------------------+
>>
>> 0:10:1473338052:uprego@uxio:~$ diff -u /usr/bin/gitk-deb8-vanilla
>> /usr/bin/gitk-deb8-multistash
>> --- /usr/bin/gitk-deb8-vanilla 2016-08-29 10:07:06.507344629 +0200
>> +++ /usr/bin/gitk-deb8-multistash 2016-09-08 14:36:35.382476634 +0200
>> @@ -401,6 +401,10 @@
>>
>> if {$vcanopt($view)} {
>> set revs [parseviewrevs $view $vrevs($view)]
>> + set stashesfd [open [concat | gitk-stash-list-ids.sh] r]
>
> set stashesfd [open [list | git stash list {--pretty=format:%H}] r]
>
>> + while {[gets $stashesfd stashline] >= 0} {
>> + set revs [lappend revs $stashline]
>> + }
>
> Could this ever take a long time? The UI is unresponsive while we're
> in this loop. If this is always quick (even on large repos), OK. If
> it could take a long time then we'll need a file event handler.
>
>> if {$revs eq {}} {
>> return 0
>> }
>> @@ -1778,7 +1782,7 @@
>> foreach v {tagids idtags headids idheads otherrefids idotherrefs} {
>> catch {unset $v}
>> }
>> - set refd [open [list | git show-ref -d] r]
>> + set refd [open [list | git show-ref -d | grep -v stash] r]
>
> If I had a branch called "dont-use-a-stash-for-this", would it get
> filtered out by this grep? It seems like it would, and we don't want
> it to. So the filtering needs to be more exact here.
>
>> while {[gets $refd line] >= 0} {
>> if {[string index $line 40] ne " "} continue
>> set id [string range $line 0 39]
>> @@ -1811,6 +1815,16 @@
>> }
>> }
>> catch {close $refd}
>> + set stashesidsrefsd [open [list | gitk-stash-list-ids-refs.sh] r]
>
> set stashesidsrefsd [open [list | \
> git stash list {--pretty=format:%H %gD}] r]
>
> Paul.
^ permalink raw reply [flat|nested] 2+ messages in thread