Skip to content
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

Change: (helm-buffer--details) Handle non-modified, non-file buffers #1917

Merged

Conversation

alphapapa
Copy link
Member

Modified, non-file buffers are now shown as modified. This is useful for, e.g. chat clients, whose room buffers are not file-backed, but it's useful to know when they're modified (i.e. have new messages).

Thanks for your work on Helm!

Modified, non-file buffers are now shown as modified.  This is useful
for, e.g. chat clients, whose room buffers are not file-backed, but it's
useful to know when they're modified (i.e. have new messages).
@alphapapa alphapapa force-pushed the buffer-details-modified-non-file branch from 0d058d5 to 994f0f3 Compare November 16, 2017 09:18
@thierryvolpiatto thierryvolpiatto merged commit 78ac22e into emacs-helm:master Nov 17, 2017
@thierryvolpiatto
Copy link
Member

Thanks merged.

@thierryvolpiatto
Copy link
Member

However, note that buffer-modified-p check if buffer have been modified against its file, but doesn't handle non-file buffers, so I am not sure this is what you want.

@alphapapa
Copy link
Member Author

Hi Thierry,

Thanks for merging.

However, note that buffer-modified-p check if buffer have been modified against its file, but doesn't handle non-file buffers, so I am not sure this is what you want.

Are you sure? I've been using set-buffer-modified-p on non-file buffers, and after setting that, buffer-modified-p returns the value set with it. IOW it seems to work as desired.

Thanks.

@thierryvolpiatto
Copy link
Member

Yes is you use set-buffer-modified-p on those buffers, but when are you setting buffer modified ?
I have reverted for now as buffers such "messages" or "eshell" stay marked as modified forever with your changes, which is confusing.

@alphapapa
Copy link
Member Author

alphapapa commented Nov 18, 2017

Yes is you use set-buffer-modified-p on those buffers, but when are you setting buffer modified ?

In our Matrix client: https://github.com/jgkamat/matrix-client-legacy-el

I have reverted for now as buffers such "messages" or "eshell" stay marked as modified forever with your changes, which is confusing.

Hmm, yeah, that's not good. That's disappointing, because having these chat room buffers show up as modified is very helpful.

The first thing I tried was setting buffer-file-name on the non-file buffers, but that caused Emacs to always use - as the buffer name when calling rename-buffer.

I feel stumped at the moment. Do you have any general ideas for how this might be fixable? I think other packages could benefit from this, like all the IRC packages.

@jgkamat What do you think?

Thanks.

@alphapapa
Copy link
Member Author

Ok, I think I figured it out: we bind buffer-file-name to nil around the call to rename-buffer. Then we can leave buffer-file-name set to "" the rest of the time, and Helm will highlight it as if it were a file-backed buffer.

Thanks, Thierry. Sorry for wasting your time with this and having to revert it.

@alphapapa
Copy link
Member Author

Well, there is still one small issue: now Helm shows them as file-backed buffers all the time. This trade-off is probably worth it, as it's much more useful to see them highlighted when they contain new messages than it is to see them as non-file-backed buffers the rest of the time. But if there is some way to have both, it would be nice. Ideas appreciated. :)

@thierryvolpiatto
Copy link
Member

What you can do is instead of using set-buffer-modified-p on your chat buffers is setting a new helm local variable to buffer-modified-tick, the variable would be named helm-buffers-tick-counter, so the only thing you would have to do on your side is (setq helm-buffers-tick-counter (buffer-modified-tick)).
Try patch below (not tested):

diff --git a/helm-buffers.el b/helm-buffers.el
index 180194ff..59ee70a6 100644
--- a/helm-buffers.el
+++ b/helm-buffers.el
@@ -154,6 +154,9 @@ this source is accessible and properly loaded."
   "Face used for non-file buffers in `helm-buffers-list'."
   :group 'helm-buffers-faces)
 
+(defvar helm-buffers-tick-counter nil)
+(make-variable-buffer-local 'helm-buffers-tick-counter)
+
 
 ;;; Buffers keymap
 ;;
@@ -375,6 +378,13 @@ See `ido-make-buffer-list' for more infos."
          (helm-buffer--show-details
           name name-prefix file-name size mode dir
           'helm-buffer-file 'helm-buffer-process nil details 'filebuf))
+        ;; A non-file, modified buffer
+        ((with-current-buffer name
+           (and helm-buffers-tick-counter
+                (/= helm-buffers-tick-counter (buffer-modified-tick))))
+         (helm-buffer--show-details
+          name (and proc name-prefix) dir size mode dir
+          'helm-buffer-modified 'helm-buffer-process proc details 'nofile-mod))
         ;; Any non--file buffer.=>italic
         (t
          (helm-buffer--show-details

@alphapapa
Copy link
Member Author

Thanks, that seems to work very well, and it's easy to gate on our end with (when (featurep 'helm).... Should I send a PR with that patch?

alphapapa added a commit to alphapapa/matrix-client.el that referenced this pull request Nov 18, 2017
alphapapa added a commit to alphapapa/matrix-client.el that referenced this pull request Nov 18, 2017
@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Nov 18, 2017 via email

thierryvolpiatto pushed a commit that referenced this pull request Nov 20, 2017
* helm-buffers.el (helm-buffers-tick-counter): New.
(helm-buffer--details): Use it.
@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Nov 20, 2017 via email

@alphapapa
Copy link
Member Author

Thanks, Thierry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants