Re: [PATCH] Gitk Inotify support

diff --git a/gitk b/gitk
index 805a1c7..58e3dca 100755
--- a/gitk
+++ b/gitk

@@ -8,6 +8,12 @@ exec wish "$0" -- "$@"
 # either version 2, or (at your option) any later version.

 package require Tk
+try {
+    package require inotify
+    set have_inotify true
+} on error {em} {
+    set have_inotify false
+}

 proc hasworktree {} {
     return [expr {[exec git rev-parse --is-bare-repository] == "false" &&

@@ -12363,6 +12369,53 @@ if {$i >= [llength $argv] && $revtreeargs ne {}} {
     }
 }

+proc inotify_handler { fd } {
+    set events [inotify_watch read]
+    set watch_info [inotify_watch info]
+    set update_view false
+
+    foreach {event} $events {
+        set current_watchid [dict get $event watchid]
+        set flags [dict get $event flags]
+        set event_filename [dict get $event filename]
+
+        foreach {path watchid watch_flags} $watch_info {
+            if {$watchid eq $current_watchid} {
+                set watch_path $path
+            }
+        }
+
+        set full_filename [file join $watch_path $event_filename]
+
+        if {$flags eq "nD"} {
+            set wd [inotify_watch add $full_filename "nwds"]
+        }
+        if {![string match *.lock $event_filename]} {
+            set update_view true
+        }
+    }
+
+    #reloadcommits or updatecommits - depending on file and operation?
+    if {$update_view} {
+        updatecommits
+    }
+}
+
+proc watch_recursive { dir } {
+    inotify_watch add $dir "nwaCmMds"
+
+    foreach i [glob -nocomplain -dir $dir *] {
+        if {[file type $i] eq {directory}} {
+            watch_recursive $i
+        }
+    }
+}
+
+if { $have_inotify } {
+    set fd [inotify create "inotify_watch" "::inotify_handler"]
+    watch_recursive $gitdir
+}
+
 set nullid "0000000000000000000000000000000000000000"
 set nullid2 "0000000000000000000000000000000000000001"
 set nullfile "/dev/null"

-- 
2.7.4

On Thu, Jun 9, 2016 at 11:24 PM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Jun 9, 2016 at 2:12 PM, Florian Schüller
> <florian.schueller@gmail.com> wrote:
>> Hi
>> Is this correct to send possible gitk patches here? or should I send
>> them to Paul Mackerras somehow?
>
> I cc'd Paul for you :)
>
>>
>> Anyway I just wanted that gitk automatically updates while I'm working
>> in my terminal
>
> Thanks for coming up with a patch. Welcome to the Git community!
>
>>
>> Are you interrested?
>>
>
> Also see the section that talks about signing off the patch and how to
> send the patch
> inline. :)
>
>
>> From 785ed6bc1b4a3b9019d3503b066afb2a025a2bc1 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Florian=20Sch=C3=BCller?= <florian.schueller@gmail.com>
>> Date: Thu, 9 Jun 2016 22:54:43 +0200
>> Subject: [PATCH] first support for inotify
>
> Here you should describe your change, i.e. what problem is solved in this patch,
> what are the alternatives, why is this way the best? Also the sign off
> goes here.
>
>>
> ---
>>  gitk | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 59 insertions(+)
>>
> diff --git a/gitk b/gitk
>> index 805a1c7..6e2ead2 100755
>> --- a/gitk
>> +++ b/gitk
>> @@ -8,6 +8,12 @@ exec wish "$0" -- "$@"
>>  # either version 2, or (at your option) any later version.
>>
>>  package require Tk
>> +try {
>> +    package require inotify
>> +    set we_have_inotify true
>> +} on error {em} {
>> +    set we_have_inotify false
>> +}
>
> There are quite a few "have_*" variables, so I would drop the leading "we_"
>
>>
>>  proc hasworktree {} {
>>      return [expr {[exec git rev-parse --is-bare-repository] == "false" &&
>> @@ -12363,6 +12369,59 @@ if {$i >= [llength $argv] && $revtreeargs ne {}} {
>>      }
>>  }
>>
>> +proc inotify_handler { fd } {
>> +    set events [inotify_watch read]
>> +    set watch_info [inotify_watch info]
>> +    set update_view false
>> +
>> +    foreach {event} $events {
>> +        set current_watchid [dict get $event watchid]
>> +        set flags [dict get $event flags]
>> +        set event_filename [dict get $event filename]
>> +
>> +        foreach {path watchid watch_flags} $watch_info {
>> +            if {$watchid eq $current_watchid} {
>> +                set watch_path $path
>> +            }
>> +        }
>> +
>> +        set full_filename [file join $watch_path $event_filename]
>> +
>> +#        remove does not seem to work
>> +#        if {$flags eq "s"} {
>> +#            puts "Remove watch $full_filename"
>> +#            set wd [inotify_watch remove $full_filename]
>> +#        }
>
> Why do we want to carry commented code? I'd drop that.
>
>> +
>> +        if {$flags eq "nD"} {
>> +            set wd [inotify_watch add $full_filename "nwds"]
>> +        }
>> +        if {![string match *.lock $event_filename]} {
>> +            set update_view true
>> +        }
>> +    }
>> +
>> +    #reloadcommits or updatecommits - depending on file and operation?
>> +    if {$update_view} {
>> +        updatecommits
>> +    }
>> +}
>> +
>> +proc watch_recursive { dir } {
>> +    inotify_watch add $dir "nwaCmMds"
>> +
>> +    foreach i [glob -nocomplain -dir $dir *] {
>> +        if {[file type $i] eq {directory}} {
>> +            watch_recursive $i
>> +        }
>> +    }
>> +}
>> +
>> +if { $we_have_inotify } {
>> +    set fd [inotify create "inotify_watch" "::inotify_handler"]
>> +    watch_recursive $gitdir
>> +}
>> +
>>  set nullid "0000000000000000000000000000000000000000"
>>  set nullid2 "0000000000000000000000000000000000000001"
>>  set nullfile "/dev/null"
>> --
>> 2.7.4
>>
>
> Thanks,
> Stefan