bug#16904: 24.3; [PATCH] ff-find-other-file and friends now work with indirect clone buffers

bug#16904: 24.3; [PATCH] ff-find-other-file and friends now work with indirect clone buffers

[Dima Kogan]

[-- Attachment #1: Type: text/plain, Size: 168 bytes --]

Attached is a small patch that allows (ff-find-other-file) to work with
indirect clone buffers. All it does is to look at the base buffer if
(buffer-file-name) is nil


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-find-file-functions-now-work-from-indirect-clone-buf.patch --]
[-- Type: text/x-diff, Size: 3747 bytes --]

From af44dfe3785c2e02f6216ecbbd15f484e1c6d8c1 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Fri, 28 Feb 2014 03:02:03 -0800
Subject: [PATCH] find-file functions now work from indirect clone buffers

---
 lisp/find-file.el | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/lisp/find-file.el b/lisp/find-file.el
index f3a135a..5992454 100644
--- a/lisp/find-file.el
+++ b/lisp/find-file.el
@@ -378,6 +378,15 @@ Variables of interest include:
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Support functions
 
+(defun ff-buffer-file-name (&optional buf)
+  "Like `buffer-file-name' but works with indirect buffers as
+well. If BUF is nil, uses the current buffer."
+  (unless buf (setq buf (current-buffer)))
+
+  (or (buffer-file-name buf)
+      (when (buffer-base-buffer buf)
+	(buffer-file-name (buffer-base-buffer buf)))))
+
 (defun ff-find-the-other-file (&optional in-other-window)
   "Find the header or source file corresponding to the current file.
 Being on a `#include' line pulls in that file, but see the help on
@@ -420,9 +429,7 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
       (setq alist (if (symbolp ff-other-file-alist)
                       (symbol-value ff-other-file-alist)
                     ff-other-file-alist)
-            pathname (if (buffer-file-name)
-                         (buffer-file-name)
-                       "/none.none"))
+            pathname (or (ff-buffer-file-name) "/none.none"))
 
       (setq fname (file-name-nondirectory pathname)
             no-match nil
@@ -448,7 +455,7 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
         ;; invoke it with the name of the current file
         (if (and (atom action) (fboundp action))
             (progn
-              (setq suffixes (funcall action (buffer-file-name))
+              (setq suffixes (funcall action (ff-buffer-file-name))
                     match (cons (car match) (list suffixes))
                     stub nil
                     default-name (car suffixes)))
@@ -550,9 +557,7 @@ the `ff-ignore-include' variable."
       (setq alist (if (symbolp ff-other-file-alist)
                       (symbol-value ff-other-file-alist)
                     ff-other-file-alist)
-            pathname (if (buffer-file-name)
-                         (buffer-file-name)
-                       "/none.none"))
+            pathname (or (ff-buffer-file-name) "/none.none"))
 
       (setq fname (file-name-nondirectory pathname)
             match (car alist))
@@ -576,7 +581,7 @@ the `ff-ignore-include' variable."
         ;; invoke it with the name of the current file
         (if (and (atom action) (fboundp action))
             (progn
-              (setq suffixes (funcall action (buffer-file-name))
+              (setq suffixes (funcall action (ff-buffer-file-name))
                     match (cons (car match) (list suffixes))
                     stub nil))
 
@@ -655,14 +660,14 @@ name of the first file found."
           (message "Finding buffer %s..." filename))
 
       (if (bufferp (get-file-buffer filename))
-          (setq found (buffer-file-name (get-file-buffer filename))))
+          (setq found (ff-buffer-file-name (get-file-buffer filename))))
 
       (setq blist (buffer-list))
       (setq buf (buffer-name (car blist)))
       (while (and blist (not found))
 
         (if (string-match-p (concat filename "<[0-9]+>") buf)
-            (setq found (buffer-file-name (car blist))))
+            (setq found (ff-buffer-file-name (car blist))))
 
         (setq blist (cdr blist))
         (setq buf (buffer-name (car blist))))
-- 
1.9.0

bug#16904: 24.3; [PATCH] ff-find-other-file and friends now work with indirect clone buffers

[Stefan Monnier]

> Attached is a small patch that allows (ff-find-other-file) to work with
> indirect clone buffers.

Could you explain exactly what you use indirect-buffers for?
These have so many shortcomings/bugs that it would be good to try and
provide good replacements,


        Stefan

bug#16904: 24.3; [PATCH] ff-find-other-file and friends now work with indirect clone buffers

[Dima Kogan]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Attached is a small patch that allows (ff-find-other-file) to work with
>> indirect clone buffers.
>
> Could you explain exactly what you use indirect-buffers for?
> These have so many shortcomings/bugs that it would be good to try and
> provide good replacements,

Hi.

I wasn't aware that use of indirect buffers is frowned-upon. Mostly
they've been working pretty well for me.

I usually use it to simultaneously look at different places in the same
file. Most of the time this is some long source code thing. It's nice to
have separate kill rings, mark rings, narrowing, etc.

The only issues I've encountered have to do with various functions not
recognizing that those buffers have a backing file. Those are
ff-find-other-file, vc-diff, etc. I suspect that making
(buffer-file-name) work for indirect buffers would resolve a lot of
these, but I don't know enough about the internals to propose that.

dima

bug#16904: 24.3; [PATCH] ff-find-other-file and friends now work with indirect clone buffers

[Stefan Monnier]

[ Replying to the bug only is fine, thanks.  CC-ing me is fine as well.
  Even sending me an extra copy like you did is fine.  I'm easy to please.  ]

> I wasn't aware that use of indirect buffers is frowned-upon.

It's not exactly frowned-upon, but it suffers from many problems/bugs
some of which are basically impossible to fix.  Part of the problem is
that this is a rarely used feature, so it's not well tested, and the
other part is that it works at a very low level and hence often lacks
higher-level info about is intended.

> I usually use it to simultaneously look at different places in the same
> file. Most of the time this is some long source code thing. It's nice to
> have separate kill rings, mark rings, narrowing, etc.

Multiple narrowing within the same buffer is indeed not supported, but
we could easily add commands to "hide everything but the current region"
which are per-window, where "hide" means "make invisible" rather than
making it completely inaccessible as narrowing does.

Not sure what you mean by separate kill rings, since kill-ring is
a global variable.

As for separate mark-rings, I'm not sure what alternative I can offer.
Maybe we could have some kind of new command `pop-to-nearest-mark'?

> The only issues I've encountered have to do with various functions not
> recognizing that those buffers have a backing file.  Those are
> ff-find-other-file, vc-diff, etc. I suspect that making
> (buffer-file-name) work for indirect buffers would resolve a lot of
> these, but I don't know enough about the internals to propose that.

Changing only the buffer-file-name function is too risky: it would make
it behave subtly differently from the buffer-file-name variable, and
since this subtlety only appears with indirect-buffers (remember:
rarely tested), it's a recipe for bugs.

Changing ff-find-other-file and friends to handle this problem is much
safer.  The problem with it is that I don't really want to go down that
slope, because there are many other such "minor bugs", depending on your
particular use case, so fixing them leads to adding a lot of support
code spread all over the place, and all this for a half-broken, rarely
used feature.


        Stefan

bug#16904: 24.3; [PATCH] ff-find-other-file and friends now work with indirect clone buffers

[Dima Kogan]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Multiple narrowing within the same buffer is indeed not supported, but
> we could easily add commands to "hide everything but the current region"
> which are per-window, where "hide" means "make invisible" rather than
> making it completely inaccessible as narrowing does.

Is the intent for this to replace narrowing? I think it would be
confusing to have two very similar but subtly different mechanisms for
doing the same thing.


> Not sure what you mean by separate kill rings, since kill-ring is
> a global variable.

Yes, you're right. Never mind on that one.


> As for separate mark-rings, I'm not sure what alternative I can offer.
> Maybe we could have some kind of new command `pop-to-nearest-mark'?

Maybe. That's brittle, though. The nearest mark COULD be in the other
window's section of the buffer, depending on exactly how the split is
set up.

Here's another way in which an indirect view into a buffer is better
than another window: iswitchb (and maybe other C-x b methods too) sorts
already-visible windows last. This usually results in way more typing
when trying to switch in this way. Here's a concrete example:

1. say you're looking at a buffer 'file' in a window that's taking up
the whole frame

2. C-x 3. Now you have two windows. Both show 'file'.

3. C-x b [enter]. We just switched to some other buffer (doesn't matter
which) in one of the windows. I'm assuming iswitchb here

4. C-x b [enter]. Same keys as before, entered again. Normally this
would switch back to the previously-active buffer, but since this buffer
is already visible, it'll default to some other buffer. You'll have to
type enough of the buffer name to uniquely identify it, before iswitchb
will recognize it.


>> The only issues I've encountered have to do with various functions not
>> recognizing that those buffers have a backing file.  Those are
>> ff-find-other-file, vc-diff, etc. I suspect that making
>> (buffer-file-name) work for indirect buffers would resolve a lot of
>> these, but I don't know enough about the internals to propose that.
>
> Changing only the buffer-file-name function is too risky: it would make
> it behave subtly differently from the buffer-file-name variable, and
> since this subtlety only appears with indirect-buffers (remember:
> rarely tested), it's a recipe for bugs.

Hmmm. You could set the buffer-file-name variable when the indirect
buffer is created. I guess I just don't know about (and haven't seen
any) fundamental breakage in indirect buffers, so my current instinct is
to try to fix it, rather than switch to something else entirely. Most of
the issues I've seen are small things, like modules that use
(buffer-file-name).


> Changing ff-find-other-file and friends to handle this problem is much
> safer.  The problem with it is that I don't really want to go down that
> slope, because there are many other such "minor bugs", depending on your
> particular use case, so fixing them leads to adding a lot of support
> code spread all over the place, and all this for a half-broken, rarely
> used feature.

Completely agree. I just don't know enough about the core to be able to
submit useful patches there, so this patch did the easy thing.

What issues are there with indirect buffers? The default C-x 4 c binding
makes an indirect clone, so it's not THAT obscure.

dima

bug#16904: 24.3; [PATCH] ff-find-other-file and friends now work with indirect clone buffers

[Stefan Monnier]

>> Multiple narrowing within the same buffer is indeed not supported, but
>> we could easily add commands to "hide everything but the current region"
>> which are per-window, where "hide" means "make invisible" rather than
>> making it completely inaccessible as narrowing does.
> Is the intent for this to replace narrowing?

Not "replace", but yes, it would be a new similar feature.

> I think it would be confusing to have two very similar but subtly
> different mechanisms for doing the same thing.

Agreed.  But if we want several separate narrowings at the same time in
a single buffer (i.e. without using indirect buffers), then I think
that's what it would look like.

I imagine it would come as an Elisp package which would replace C-x n n,
for those rare users like you who want such a functionality.

>> As for separate mark-rings, I'm not sure what alternative I can offer.
>> Maybe we could have some kind of new command `pop-to-nearest-mark'?
> Maybe. That's brittle, though.

Yes, clearly.

> Here's another way in which an indirect view into a buffer is better
> than another window: iswitchb (and maybe other C-x b methods too) sorts
> already-visible windows last. This usually results in way more typing
> when trying to switch in this way.

Interesting point, indeed.

> Hmmm. You could set the buffer-file-name variable when the indirect
> buffer is created.

It's set to nil to avoid problems linked to having to handle "multiple
buffers accessing the same file".  We'd otherwise have to add special
code so that (for example) when you save one of the buffers, the
others's states are correspondingly updated (otherwise they'd think that
some external process changed the file).

> I guess I just don't know about (and haven't seen any) fundamental
> breakage in indirect buffers, so my current instinct is to try to fix
> it, rather than switch to something else entirely.  Most of the issues
> I've seen are small things, like modules that use (buffer-file-name).

There's some major breakage lurking with most modes that use
things like text-properties or after-change-functions.

A way to "fix" things might be to restrict the use of indirect clones so
they are read-only.

> What issues are there with indirect buffers? The default C-x 4 c binding
> makes an indirect clone, so it's not THAT obscure.

I know.  It seemed like a good idea at the time ;-(


        Stefan

bug#16904: 24.3; [PATCH] ff-find-other-file and friends now work with indirect clone buffers

[Lars Ingebrigtsen]

Dima Kogan <dima@secretsauce.net> writes:

> Attached is a small patch that allows (ff-find-other-file) to work with
> indirect clone buffers. All it does is to look at the base buffer if
> (buffer-file-name) is nil

Thanks; applied to the trunk.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no