New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File viewer refreshes too often #517

Closed
cova-fe opened this Issue Nov 2, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@cova-fe

cova-fe commented Nov 2, 2015

I'm noticing a quite annoying behaviour in file-viewer. Basically, file lsit is refreshed a lot of times, probably even without need. If I open a file with ctrl-e, file list is refreshed and it is (at least on my system) a slow operation,as the whole window is redrawn. When the file is closed, another refresh is performed and so on.

Maybe git-cola is monitoring too many flags of inotify or something like that?

I don't know if this a general issue or only on my system; with some hint I could also dig into this on my own.

git-cola fetched from github (bb439b3)
python 3.4

built using gentoo system

@living180

This comment has been minimized.

Show comment
Hide comment
@living180

living180 Nov 2, 2015

Contributor

I suspect that this behavior has always been present in git-cola's filesystem monitoring code, but you are probably seeing it now due to some (still unreleased) changes I made to the implementation. Prior versions of git-cola implemented filesystem monitoring on Linux using pyinotify. With my changes, it no longer uses pyinotify but instead invokes the inotify functions from libc directly via ctypes. As a result, filesystem monitoring now enabled by default on Linux, when previously it would only be enabled if pyinotify was installed.

What editor do you use? Does it create a temporary file alongside your source file when you edit it (à la vim's .swp file?). The creation (or deletion, when closing the source file) of such temporary file will trigger a refresh. I suspect this may be the source of the problem. It may be possible to add some intelligence to the filesystem monitoring code to avoid performing a refresh if the file in question is ignored by git.

In the interim, you also set cola.inotify to false in your .gitconfig to prevent git-cola from refreshing on filesystem changes.

Contributor

living180 commented Nov 2, 2015

I suspect that this behavior has always been present in git-cola's filesystem monitoring code, but you are probably seeing it now due to some (still unreleased) changes I made to the implementation. Prior versions of git-cola implemented filesystem monitoring on Linux using pyinotify. With my changes, it no longer uses pyinotify but instead invokes the inotify functions from libc directly via ctypes. As a result, filesystem monitoring now enabled by default on Linux, when previously it would only be enabled if pyinotify was installed.

What editor do you use? Does it create a temporary file alongside your source file when you edit it (à la vim's .swp file?). The creation (or deletion, when closing the source file) of such temporary file will trigger a refresh. I suspect this may be the source of the problem. It may be possible to add some intelligence to the filesystem monitoring code to avoid performing a refresh if the file in question is ignored by git.

In the interim, you also set cola.inotify to false in your .gitconfig to prevent git-cola from refreshing on filesystem changes.

@cova-fe

This comment has been minimized.

Show comment
Hide comment
@cova-fe

cova-fe Nov 2, 2015

yep, right on the spot :) I'm using vim/qvim that make use of .swp files. I'll follow your advice about cola.inotify. Ignoring the same files ignored by git could be a good idea, imho. Or otherwise allow the user to specify a cola-specific blacklist, maybe defaulting to gitignore.
Many thanks for your answer!

cova-fe commented Nov 2, 2015

yep, right on the spot :) I'm using vim/qvim that make use of .swp files. I'll follow your advice about cola.inotify. Ignoring the same files ignored by git could be a good idea, imho. Or otherwise allow the user to specify a cola-specific blacklist, maybe defaulting to gitignore.
Many thanks for your answer!

living180 added a commit to living180/git-cola that referenced this issue May 12, 2016

fsmonitor: avoid refreshing for ignored files
Changes to files ignored by git won't cause display changes in git-cola,
but they were still causing the file system change monitoring to refresh
the display.  This commit avoids this behavior by using git check-ignore
with if git version 1.8.5 or later is present.  The logic in the file
system change monitor is enhanced so that it no longer triggers a
refresh on any filesystem change.  Now, if the change is for a file, it
adds the file path to a set of paths that gets checked in the
_BaseThread.notify() method.  This method passes the paths to git
check-ignore, and only triggers a refresh if at least one of the paths
is for a non-ignored file.

Fixes: #517
Related-to: #516

Reported-by: Fabio Coatti <fabio.coatti@gmail.com>
Reported-by: Stanisław Halik <sthalik@misaki.pl>
Signed-off-by: Daniel Harding <dharding@living180.net>

living180 added a commit to living180/git-cola that referenced this issue May 12, 2016

fsmonitor: avoid refreshing for ignored files
Changes to files ignored by git won't cause display changes in git-cola,
but they were still causing the file system change monitoring to refresh
the display.  This commit avoids this behavior by using git check-ignore
with if git version 1.8.5 or later is present.  The logic in the file
system change monitor is enhanced so that it no longer triggers a
refresh on any filesystem change.  Now, if the change is for a file, it
adds the file path to a set of paths that gets checked in the
_BaseThread.notify() method.  This method passes the paths to git
check-ignore, and only triggers a refresh if at least one of the paths
is for a non-ignored file.

Fixes: #517
Related-to: #516

Reported-by: Fabio Coatti <fabio.coatti@gmail.com>
Reported-by: Stanisław Halik <sthalik@misaki.pl>
Signed-off-by: Daniel Harding <dharding@living180.net>

sthalik added a commit to sthalik/git-cola that referenced this issue Aug 21, 2016

fsmonitor: avoid refreshing for ignored files
Changes to files ignored by git won't cause display changes in git-cola,
but they were still causing the file system change monitoring to refresh
the display.  This commit avoids this behavior by using git check-ignore
with if git version 1.8.5 or later is present.  The logic in the file
system change monitor is enhanced so that it no longer triggers a
refresh on any filesystem change.  Now, if the change is for a file, it
adds the file path to a set of paths that gets checked in the
_BaseThread.notify() method.  This method passes the paths to git
check-ignore, and only triggers a refresh if at least one of the paths
is for a non-ignored file.

Fixes: #517
Related-to: #516

Reported-by: Fabio Coatti <fabio.coatti@gmail.com>
Reported-by: Stanisław Halik <sthalik@misaki.pl>
Signed-off-by: Daniel Harding <dharding@living180.net>

sthalik added a commit to sthalik/git-cola that referenced this issue Aug 21, 2016

fsmonitor: avoid refreshing for ignored files
Changes to files ignored by git won't cause display changes in git-cola,
but they were still causing the file system change monitoring to refresh
the display.  This commit avoids this behavior by using git check-ignore
with if git version 1.8.5 or later is present.  The logic in the file
system change monitor is enhanced so that it no longer triggers a
refresh on any filesystem change.  Now, if the change is for a file, it
adds the file path to a set of paths that gets checked in the
_BaseThread.notify() method.  This method passes the paths to git
check-ignore, and only triggers a refresh if at least one of the paths
is for a non-ignored file.

Fixes: #517
Related-to: #516

Reported-by: Fabio Coatti <fabio.coatti@gmail.com>
Reported-by: Stanisław Halik <sthalik@misaki.pl>
Signed-off-by: Daniel Harding <dharding@living180.net>

sthalik added a commit to sthalik/git-cola that referenced this issue Aug 21, 2016

fsmonitor: avoid refreshing for ignored files
Changes to files ignored by git won't cause display changes in git-cola,
but they were still causing the file system change monitoring to refresh
the display.  This commit avoids this behavior by using git check-ignore
with if git version 1.8.5 or later is present.  The logic in the file
system change monitor is enhanced so that it no longer triggers a
refresh on any filesystem change.  Now, if the change is for a file, it
adds the file path to a set of paths that gets checked in the
_BaseThread.notify() method.  This method passes the paths to git
check-ignore, and only triggers a refresh if at least one of the paths
is for a non-ignored file.

Fixes: #517
Related-to: #516

Reported-by: Fabio Coatti <fabio.coatti@gmail.com>
Reported-by: Stanisław Halik <sthalik@misaki.pl>
Signed-off-by: Daniel Harding <dharding@living180.net>
@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Sep 9, 2016

Member

Thanks @cova-fe @living180 @sthalik I'll be merging this to master shortly

Member

davvid commented Sep 9, 2016

Thanks @cova-fe @living180 @sthalik I'll be merging this to master shortly

@davvid davvid closed this in 3be2748 Sep 9, 2016

davvid added a commit that referenced this issue Sep 9, 2016

Merge remote-tracking branch 'sthalik/fsmonitor_check_ignore'
* sthalik/fsmonitor_check_ignore:
fsmonitor: fix signal usage
fsmonitor: avoid refreshing for ignored files
fsmonitor: ignore directory changes
fsmonitor: do less work if refresh is pending
fsmonitor: map watch descriptors to paths on Linux
fsmonitor: remove the refs_only parameter

Closes #516
Closes #517

Rebased-by: Stanislaw Halik <sthalik@misaki.pl>
Signed-off-by: David Aguilar <davvid@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment