proposition about bookmark-buffer-file-name in bookmark.el

proposition about bookmark-buffer-file-name in bookmark.el

[Toru TSUNEYOSHI]

Hello.

I have a proposition.
(I use GNU Emacs 22.3.1 (i386-msvc-nt5.0.2195).)

Now, `bookmark-buffer-file-name' in bookmark.el do
`abbreviate-file-name' buffer-file-name only.
So, I wish this function do `abbreviate-file-name' dired-directory (and
Info-current-file) on next version of Emacs.

`bookmark-buffer-file-name' in bookmark.el on Emacs 22.3.1:
========================================================================
(defun bookmark-buffer-file-name ()
  "Return the current buffer's file in a way useful for bookmarks.
For example, if this is a Info buffer, return the Info file's name."
  (cond
   ((eq major-mode 'Info-mode)
    Info-current-file)
   (buffer-file-name
    ;; Abbreviate the path, both so it's shorter and so it's more
    ;; portable.  E.g., the user's home dir might be a different
    ;; path on different machines, but "~/" will still reach it.
    (abbreviate-file-name buffer-file-name))
   ((and (boundp 'dired-directory) dired-directory)
    (if (stringp dired-directory)
        dired-directory
      (car dired-directory)))))
========================================================================

Re: proposition about bookmark-buffer-file-name in bookmark.el

[Karl Fogel]

"Toru TSUNEYOSHI" <t_tuneyosi@hotmail.com> writes:
> I have a proposition.
> (I use GNU Emacs 22.3.1 (i386-msvc-nt5.0.2195).)
>
> Now, `bookmark-buffer-file-name' in bookmark.el do
> `abbreviate-file-name' buffer-file-name only.
> So, I wish this function do `abbreviate-file-name' dired-directory (and
> Info-current-file) on next version of Emacs.

This sounds reasonable to me, but could you give examples of what the
new names would look like for those kinds of bookmarks?

(By the way, expressing the proposed change in 'patch' format is
probably the most precise way to communicate it.)

Thanks,
-Karl

> `bookmark-buffer-file-name' in bookmark.el on Emacs 22.3.1:
> ========================================================================
> (defun bookmark-buffer-file-name ()
>   "Return the current buffer's file in a way useful for bookmarks.
> For example, if this is a Info buffer, return the Info file's name."
>   (cond
>    ((eq major-mode 'Info-mode)
>     Info-current-file)
>    (buffer-file-name
>     ;; Abbreviate the path, both so it's shorter and so it's more
>     ;; portable.  E.g., the user's home dir might be a different
>     ;; path on different machines, but "~/" will still reach it.
>     (abbreviate-file-name buffer-file-name))
>    ((and (boundp 'dired-directory) dired-directory)
>     (if (stringp dired-directory)
>         dired-directory
>       (car dired-directory)))))
> ========================================================================

Re: proposition about bookmark-buffer-file-name in bookmark.el

[Toru TSUNEYOSHI]

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

Thank you for replying.

----- Original Message ----- 
From: "Karl Fogel" <kfogel@red-bean.com>
To: "Toru TSUNEYOSHI" <t_tuneyosi@hotmail.com>
Cc: <emacs-devel@gnu.org>
Sent: Wednesday, November 19, 2008 4:37 PM
Subject: Re: proposition about bookmark-buffer-file-name in bookmark.el


> "Toru TSUNEYOSHI" <t_tuneyosi@hotmail.com> writes:
> > I have a proposition.
> > (I use GNU Emacs 22.3.1 (i386-msvc-nt5.0.2195).)
> >
> > Now, `bookmark-buffer-file-name' in bookmark.el do
> > `abbreviate-file-name' buffer-file-name only.
> > So, I wish this function do `abbreviate-file-name' dired-directory (and
> > Info-current-file) on next version of Emacs.
> 
> This sounds reasonable to me, but could you give examples of what the
> new names would look like for those kinds of bookmarks?

Sorry, I can't understand you clearly.

Anyway, I wish that `bookmark-buffer-file-name' returns dired-directory
relative to `~/'.

examples of `bookmark-buffer-file-name' processing:

  case:
    home directory:  "d:/home/user/"
    dired-directory: "d:/home/user/xyz/"

    (bookmark-buffer-file-name)
    now
      => "d:/home/user/xyz/"
    wish
      => "~/xyz/"

> 
> (By the way, expressing the proposed change in 'patch' format is
> probably the most precise way to communicate it.)
> 
> Thanks,
> -Karl

Yes.
I use `defadvice' in practice.
But I made a patch for bookmark.el temporarily.

[-- Attachment #2: bookmark.el.diff --]
[-- Type: application/octet-stream, Size: 1391 bytes --]

--- bookmark.el.original	2008-01-07 11:45:03.000000000 +0900
+++ bookmark.el	2008-11-19 20:45:26.331974400 +0900
@@ -1007,18 +1007,20 @@
 (defun bookmark-buffer-file-name ()
   "Return the current buffer's file in a way useful for bookmarks.
 For example, if this is a Info buffer, return the Info file's name."
-  (cond
-   ((eq major-mode 'Info-mode)
-    Info-current-file)
-   (buffer-file-name
-    ;; Abbreviate the path, both so it's shorter and so it's more
-    ;; portable.  E.g., the user's home dir might be a different
-    ;; path on different machines, but "~/" will still reach it.
-    (abbreviate-file-name buffer-file-name))
-   ((and (boundp 'dired-directory) dired-directory)
-    (if (stringp dired-directory)
-        dired-directory
-      (car dired-directory)))))
+  (let (file)
+    (setq file (cond
+		((eq major-mode 'Info-mode)
+		 Info-current-file)
+		(buffer-file-name
+		 ;; Abbreviate the path, both so it's shorter and so it's more
+		 ;; portable.  E.g., the user's home dir might be a different
+		 ;; path on different machines, but "~/" will still reach it.
+		 buffer-file-name)
+		((and (boundp 'dired-directory) dired-directory)
+		 (if (stringp dired-directory)
+		     dired-directory
+		   (car dired-directory)))))
+    (and file (abbreviate-file-name file))))
 
 
 (defun bookmark-maybe-load-default-file ()

Re: proposition about bookmark-buffer-file-name in bookmark.el

[Karl Fogel]

"Toru TSUNEYOSHI" <t_tuneyosi@hotmail.com> writes:
>> This sounds reasonable to me, but could you give examples of what the
>> new names would look like for those kinds of bookmarks?
>
> Sorry, I can't understand you clearly.

What I meant was, could you describe the effect your change will have?
I mean the effect that will be visible to users.

> examples of `bookmark-buffer-file-name' processing:
>
>   case:
>     home directory:  "d:/home/user/"
>     dired-directory: "d:/home/user/xyz/"
>
>     (bookmark-buffer-file-name)
>     now
>       => "d:/home/user/xyz/"
>     wish
>       => "~/xyz/"

This looks good to me!

>> (By the way, expressing the proposed change in 'patch' format is
>> probably the most precise way to communicate it.)
>> 
>> Thanks,
>> -Karl
>
> Yes.
> I use `defadvice' in practice.
> But I made a patch for bookmark.el temporarily.

Can you post that patch?

Thanks,
-Karl

Re: proposition about bookmark-buffer-file-name in bookmark.el

[Toru TSUNEYOSHI]

----- Original Message ----- 
From: "Karl Fogel" <kfogel@red-bean.com>
To: "Toru TSUNEYOSHI" <t_tuneyosi@hotmail.com>
Cc: <emacs-devel@gnu.org>
Sent: Thursday, November 20, 2008 3:21 AM
Subject: Re: proposition about bookmark-buffer-file-name in bookmark.el


> "Toru TSUNEYOSHI" <t_tuneyosi@hotmail.com> writes:
> >> This sounds reasonable to me, but could you give examples of what the
> >> new names would look like for those kinds of bookmarks?
> >
> > Sorry, I can't understand you clearly.
> 
> What I meant was, could you describe the effect your change will have?
> I mean the effect that will be visible to users.
> 

Thanks. I have understood clearly.

The effect is as same as description about `bookmark-buffer-file-name'
in bookmark.el on Emacs 22.3.1. In short, "shorter and portable".

comment about `bookmark-buffer-file-name' in bookmark.el on Emacs 22.3.1:
========================================================================
   (buffer-file-name
    ;; Abbreviate the path, both so it's shorter and so it's more
    ;; portable.  E.g., the user's home dir might be a different
    ;; path on different machines, but "~/" will still reach it.
    (abbreviate-file-name buffer-file-name))
========================================================================

> > examples of `bookmark-buffer-file-name' processing:
> >
> >   case:
> >     home directory:  "d:/home/user/"
> >     dired-directory: "d:/home/user/xyz/"
> >
> >     (bookmark-buffer-file-name)
> >     now
> >       => "d:/home/user/xyz/"
> >     wish
> >       => "~/xyz/"
> 
> This looks good to me!
> 
> >> (By the way, expressing the proposed change in 'patch' format is
> >> probably the most precise way to communicate it.)
> >> 
> >> Thanks,
> >> -Karl
> >
> > Yes.
> > I use `defadvice' in practice.
> > But I made a patch for bookmark.el temporarily.
> 
> Can you post that patch?
> 

I have already attached it in my previous email. Please check it.

> Thanks,
> -Karl
> 

Re: proposition about bookmark-buffer-file-name in bookmark.el

[Stefan Monnier]

> I have already attached it in my previous email. Please check it.

It looks good, indeed, thanks.


        Stefan

Re: proposition about bookmark-buffer-file-name in bookmark.el

[Karl Fogel]

"Toru TSUNEYOSHI" <t_tuneyosi@hotmail.com> writes:
> Thanks. I have understood clearly.
>
> The effect is as same as description about `bookmark-buffer-file-name'
> in bookmark.el on Emacs 22.3.1. In short, "shorter and portable".
>
> comment about `bookmark-buffer-file-name' in bookmark.el on Emacs 22.3.1:
> ========================================================================
>    (buffer-file-name
>     ;; Abbreviate the path, both so it's shorter and so it's more
>     ;; portable.  E.g., the user's home dir might be a different
>     ;; path on different machines, but "~/" will still reach it.
>     (abbreviate-file-name buffer-file-name))
> ========================================================================

Thanks, I've looked at the patch now.  A few comments:

   1. It applies to some earlier version of bookmark.el, but not to the
      current version.  You can get the current version here:

      http://cvs.savannah.gnu.org/viewvc/emacs/lisp/bookmark.el?root=emacs&view=markup

   2. In the current version, I think there is no need to abbreviate the
      file name in `bookmark-buffer-file-name', as
      `bookmark-buffer-name' uses `file-name-nondirectory' anyway.  See
      the code path starting from `bookmark-set'.  So, there seems to be
      no behavior difference with the patch applied (at least as I tested).

Regarding your patch itself, a couple of comments:

   --- bookmark.el	2008-01-07 11:45:03.000000000 +0900
   +++ bookmark.el	2008-11-19 20:45:26.331974400 +0900
   @@ -1007,18 +1007,20 @@
    (defun bookmark-buffer-file-name ()
      "Return the current buffer's file in a way useful for bookmarks.
    For example, if this is a Info buffer, return the Info file's name."
   -  (cond
   -   ((eq major-mode 'Info-mode)
   -    Info-current-file)
   -   (buffer-file-name
   -    ;; Abbreviate the path, both so it's shorter and so it's more
   -    ;; portable.  E.g., the user's home dir might be a different
   -    ;; path on different machines, but "~/" will still reach it.
   -    (abbreviate-file-name buffer-file-name))
   -   ((and (boundp 'dired-directory) dired-directory)
   -    (if (stringp dired-directory)
   -        dired-directory
   -      (car dired-directory)))))
   +  (let (file)
   +    (setq file (cond
   +            ((eq major-mode 'Info-mode)
   +             Info-current-file)
   +            (buffer-file-name
   +             ;; Abbreviate the path, both so it's shorter and so it's more
   +             ;; portable.  E.g., the user's home dir might be a different
   +             ;; path on different machines, but "~/" will still reach it.
   +             buffer-file-name)
   +            ((and (boundp 'dired-directory) dired-directory)
   +             (if (stringp dired-directory)
   +                 dired-directory
   +               (car dired-directory)))))
   +    (and file (abbreviate-file-name file))))

First, instead of

   (let (file)
     (setq file ...))

it's more direct to just do this:

   (let ((file ...))

Second, when moving the `abbreviate-file-name' call, we should move the
comment that goes with it too.

However, in the new code, the `cond' structure falls through to an error
now.  That means there is no need for a `let' at all.  Here is a revised
version of the patch, for the current bookmark.el:

   2008-11-21  Karl Fogel  <kfogel@red-bean.com>
   
        * bookmark.el (bookmark-buffer-file-name): Abbreviate the result
        unconditionally.
        Suggested by Toru TSUNEYOSHI <t_tuneyosi {_AT_} hotmail.com>.
   
   Index: bookmark.el
   ===================================================================
   RCS file: /sources/emacs/emacs/lisp/bookmark.el,v
   retrieving revision 1.120
   diff -u -r1.120 bookmark.el
   --- bookmark.el	21 Nov 2008 10:32:41 -0000	1.120
   +++ bookmark.el	21 Nov 2008 16:37:30 -0000
   @@ -888,18 +888,17 @@
    
    (defun bookmark-buffer-file-name ()
      "Return the current buffer's file in a way useful for bookmarks."
   -  (cond
   -   (buffer-file-name
   -    ;; Abbreviate the path, both so it's shorter and so it's more
   -    ;; portable.  E.g., the user's home dir might be a different
   -    ;; path on different machines, but "~/" will still reach it.
   -    (abbreviate-file-name buffer-file-name))
   -   ((and (boundp 'dired-directory) dired-directory)
   -    (if (stringp dired-directory)
   -        dired-directory
   -      (car dired-directory)))
   -   (t (error "Buffer not visiting a file or directory"))))
   -
   +  ;; Abbreviate the path, both so it's shorter and so it's more
   +  ;; portable.  E.g., the user's home dir might be a different
   +  ;; path on different machines, but "~/" will still reach it.
   +  (abbreviate-file-name 
   +   (cond
   +    (buffer-file-name buffer-file-name)
   +    ((and (boundp 'dired-directory) dired-directory)
   +     (if (stringp dired-directory)
   +         dired-directory
   +       (car dired-directory)))
   +    (t (error "Buffer not visiting a file or directory")))))
    
    (defun bookmark-maybe-load-default-file ()
      (and (not bookmarks-already-loaded)

The above is just for demonstration.  Again, I don't believe the patch
is necessary, but maybe you will point out some case I didn't think of.

Thank you,
-Karl

Re: proposition about bookmark-buffer-file-name in bookmark.el

[Toru TSUNEYOSHI]

Thanks.

Your code looks like good.

(I write again, ...)
`abbreviate-file-name' is applied to `buffer-file-name',
then `abbreviate-file-name' should be applied to `dired-directory',
I thought simply.

So, your revised version looks right.

Thanks you  (and evryone), again.

Re: proposition about bookmark-buffer-file-name in bookmark.el

[Stefan Monnier]

> So, I wish this function do `abbreviate-file-name' dired-directory (and
> Info-current-file) on next version of Emacs.

Done, thank you,


        Stefan