unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
@ 2020-10-12 14:26 Boruch Baum
  2020-10-13  3:49 ` Richard Stallman
  0 siblings, 1 reply; 12+ messages in thread
From: Boruch Baum @ 2020-10-12 14:26 UTC (permalink / raw)
  To: 43961

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

The attached patch takes a step to remove confusion surrounding two
unfortunately named functions, fixes bugs in one of them, and makes them
more user-friendly. If this is all acceptable, let me know and I'll add
a NEWS entry.

+ dired-filename-at-point is moved from dired-x.el to a position
  alongside its sister function dired-file-name-at-point.

+ The code for dired-filename-at-point is simplified to an almost
  carbon copy of its sister function.

  + This also fixes bugs for cases of filenames with embedded spaces

  + This also suppresses errors, due to use of dired-get-filename with
    its arg NOERROR.

+ Both get a small style tweak to remove duplicated function calls to
  abbrev- or expand- file-name.

+ Both get aliases to make them distinguishable.

A follow-up to the attached patch, if accepted, would be to refactor all
the occurrences within dired.el / dired-x.el.

It would probably be a good idea to also add a deprecation notice to the
docstrings for the original names.

I came across this while developing the diredc package I've been
mentioning a lot lately, and presenting this as a separate patch removes
it from being snagged as part of discussion about that other proposal.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[-- Attachment #2: dired-name-confusion.patch --]
[-- Type: text/x-diff, Size: 3640 bytes --]

diff --git a/dired-x.el b/dired-x.el
index b09ef90..d3b9dc9 100644
--- a/dired-x.el
+++ b/dired-x.el
@@ -1482,37 +1482,6 @@ a prefix argument, when it offers the filename near point as a default."

 ;;; Internal functions.

-;; Fixme: This should probably use `thing-at-point'.  -- fx
-(defun dired-filename-at-point ()
-  "Return the filename closest to point, expanded.
-Point should be in or after a filename."
-  (save-excursion
-    ;; First see if just past a filename.
-    (or (eobp)                             ; why?
-        (when (looking-at-p "[] \t\n[{}()]") ; whitespace or some parens
-          (skip-chars-backward " \n\t\r({[]})")
-          (or (bobp) (backward-char 1))))
-    (let ((filename-chars "-.[:alnum:]_/:$+@")
-          start prefix)
-      (if (looking-at-p (format "[%s]" filename-chars))
-          (progn
-            (skip-chars-backward filename-chars)
-            (setq start (point)
-                  prefix
-                  ;; This is something to do with ange-ftp filenames.
-                  ;; It convert foo@bar to /foo@bar.
-                  ;; But when does the former occur in dired buffers?
-		  (and (string-match-p
-			"^\\w+@"
-			(buffer-substring start (line-end-position)))
-		       "/"))
-            (if (string-match-p "[/~]" (char-to-string (preceding-char)))
-                (setq start (1- start)))
-            (skip-chars-forward filename-chars))
-        (error "No file found around point!"))
-      ;; Return string.
-      (expand-file-name (concat prefix (buffer-substring start (point)))))))
-
 (defun dired-x-read-filename-at-point (prompt)
   "Return filename prompting with PROMPT with completion.
 If `current-prefix-arg' is non-nil, uses name at point as guess."
diff --git a/dired.el b/dired.el
index 2ecd6bd..69aa41d 100644
--- a/dired.el
+++ b/dired.el
@@ -958,16 +958,31 @@ ERROR can be a string with the error message."
 ;;             (read-file-name (format "Dired %s(directory): " str)
 ;;                             nil default-directory nil))))))))

-(defun dired-file-name-at-point ()
-  "Try to get a file name at point in the current dired buffer.
-This hook is intended to be put in `file-name-at-point-functions'.
-Note that it returns an abbreviated name that can't be used
-as an argument to `dired-goto-file'."
+(defun dired-filename-at-point-abbrev ()
+  "Return the filename closest to point, abbreviated.
+Point should be in or after a filename. Do not confuse this
+function with `dired-filename-at-point', which expands
+the file name."
   (let ((filename (dired-get-filename nil t)))
     (when filename
-      (if (file-directory-p filename)
-	  (file-name-as-directory (abbreviate-file-name filename))
-	(abbreviate-file-name filename)))))
+      (abbreviate-file-name
+        (if (file-directory-p filename)
+	  (file-name-as-directory filename)
+	 filename)))))
+(defalias 'dired-file-name-at-point 'dired-filename-at-point-abbrev)
+
+(defun dired-filename-at-point-expand ()
+  "Return the filename closest to point, expanded.
+Point should be in or after a filename. Do not confuse this
+function with `dired-file-name-at-point', which abbreviates
+the file name."
+  (let ((filename (dired-get-filename nil t)))
+    (when filename
+      (expand-file-name
+        (if (file-directory-p filename)
+	  (file-name-as-directory filename)
+	 filename)))))
+(defalias 'dired-filename-at-point 'dired-filename-at-point-expand)

 (defun dired-grep-read-files ()
   "Use file at point as the file for grep's default file-name pattern suggestion.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-12 14:26 bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point Boruch Baum
@ 2020-10-13  3:49 ` Richard Stallman
  2020-10-13  4:08   ` Boruch Baum
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Stallman @ 2020-10-13  3:49 UTC (permalink / raw)
  To: Boruch Baum; +Cc: 43961

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > + dired-filename-at-point is moved from dired-x.el to a position
  >   alongside its sister function

What is the reason for keeping that function at all?
What makes it usefully different from
dired-file-name-at-point?

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)







^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-13  3:49 ` Richard Stallman
@ 2020-10-13  4:08   ` Boruch Baum
  2020-10-13  4:54     ` Lars Ingebrigtsen
  2020-10-14  4:43     ` Richard Stallman
  0 siblings, 2 replies; 12+ messages in thread
From: Boruch Baum @ 2020-10-13  4:08 UTC (permalink / raw)
  To: Richard Stallman; +Cc: 43961

On 2020-10-12 23:49, Richard Stallman wrote:
>   > + dired-filename-at-point is moved from dired-x.el to a position
>   >   alongside its sister function
>
> What is the reason for keeping that function at all?

I couldn't possibly account for all the possible emacs users who may
have written private extensions over the years using one or the other of
the two functions, so in order to be courteous and not break anyone's
personal emacs I left both original symbol names extant and suggested a
deprecation warning that could be followed in a later version with an EOL.

As I think I mentioned, once (if) the patch is approved, a next step is
to grep the emacs core code-base and replace the function names.

> What makes it usefully different from dired-file-name-at-point?

They return different values. One returns an expanded (canonical)
path-name, and the other an abbreviated one.

This quickly becomes an emacs history/archaeology exercise (not my PhD):
the functions were written by different people at different times for
different packages. One function entered emacs core when package dired-x
(1993-, Sebastian Kremer) was integrated. My hunch is that modifications
and bug fixes over time were only applied the dired.el version.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-13  4:08   ` Boruch Baum
@ 2020-10-13  4:54     ` Lars Ingebrigtsen
  2020-10-13 10:25       ` Boruch Baum
  2020-10-14  4:43     ` Richard Stallman
  1 sibling, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2020-10-13  4:54 UTC (permalink / raw)
  To: Boruch Baum; +Cc: 43961, Richard Stallman

Boruch Baum <boruch_baum@gmx.com> writes:

> They return different values. One returns an expanded (canonical)
> path-name, and the other an abbreviated one.

Why not just have one call the other, and wrap the results in
abbreviate-file-name?

(And then mark it as obsolete, since it doesn't seem very useful.)

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-13  4:54     ` Lars Ingebrigtsen
@ 2020-10-13 10:25       ` Boruch Baum
  2020-10-13 10:58         ` Arthur Miller
  2020-10-14  3:45         ` Lars Ingebrigtsen
  0 siblings, 2 replies; 12+ messages in thread
From: Boruch Baum @ 2020-10-13 10:25 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 43961, Richard Stallman

On 2020-10-13 06:54, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> > They return different values. One returns an expanded (canonical)
> > path-name, and the other an abbreviated one.
>
> Why not just have one call the other, and wrap the results in
> abbreviate-file-name?

You could, but you wouldn't be saving anything since the inner function
would still need to perform the expansion, so for the abbreviated
function you end up in effect with an inefficient (abbrev (expand file))
instead of a choice between (abbrev file) or (expand file).

Also, much of the change ends up being defaliases, docstrings and
deprecation notices, so its more clearly presented without nesting
functions.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-13 10:25       ` Boruch Baum
@ 2020-10-13 10:58         ` Arthur Miller
  2020-10-13 11:27           ` Boruch Baum
  2020-10-14  3:45         ` Lars Ingebrigtsen
  1 sibling, 1 reply; 12+ messages in thread
From: Arthur Miller @ 2020-10-13 10:58 UTC (permalink / raw)
  To: Boruch Baum; +Cc: 43961, Lars Ingebrigtsen, Richard Stallman

Boruch Baum <boruch_baum@gmx.com> writes:

> On 2020-10-13 06:54, Lars Ingebrigtsen wrote:
>> Boruch Baum <boruch_baum@gmx.com> writes:
>>
>> > They return different values. One returns an expanded (canonical)
>> > path-name, and the other an abbreviated one.
>>
>> Why not just have one call the other, and wrap the results in
>> abbreviate-file-name?
>
> You could, but you wouldn't be saving anything since the inner function
> would still need to perform the expansion, so for the abbreviated
> function you end up in effect with an inefficient (abbrev (expand file))
> instead of a choice between (abbrev file) or (expand file).
>
> Also, much of the change ends up being defaliases, docstrings and
> deprecation notices, so its more clearly presented without nesting
> functions.
>
> --
> hkp://keys.gnupg.net
> CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0
Boruch you are mind reader! At least I thought so when I saw your mail
yesterday.

I was playing with Dired myself on Monday and I actually wrote a mail
about those two functions + file-name-directory and directory-file-name,
I just never send it.

dired-filename-at-point is supposed to return a filename closest to the
point, according to docs. For me it didn't work at all.

But I mostly disliked the name. Why are there two almost identical name
for different functionality? I like self-documented names for variables
but if there are names like directory-file-name and file-name-directory
or something like dired-file-name-at-point and dired-filename-at-point;
then I have to remember and actively think about which one was that I
want? Usually ends up with unnecessary look-up into docs, because I
don't remember which one was that I want. Not that I hate so much to
think :-), but it does interrupt the flow of thoughts.

I understand that logic was 'objectoworkon-followed-by-operation' but I
still find it unnecessary convoluted. It is just so much more
straightforward if each function has unique descriptive name, and yes I
am aware that such names are sometimes hard to get by.

I have seen also discussion about string-replace and replace-string, can
we plase not? For the reason above. And for the new APIs added, please
don't use naming patterns like word1-word2 and word2-word1 or similar
where same words are used just with some slight variation.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-13 10:58         ` Arthur Miller
@ 2020-10-13 11:27           ` Boruch Baum
  0 siblings, 0 replies; 12+ messages in thread
From: Boruch Baum @ 2020-10-13 11:27 UTC (permalink / raw)
  To: Arthur Miller; +Cc: 43961, Lars Ingebrigtsen, Richard Stallman

On 2020-10-13 12:58, Arthur Miller wrote:
> ...
> + file-name-directory and directory-file-name,

Another excellent example that I always find confusing, even after much
dired programming.

> dired-filename-at-point is supposed to return a filename closest to the
> point, according to docs. For me it didn't work at all.

I seems to not have been as well-maintained as its sister function,
possibly because they've been in separate files and the maintainer
forgot about one. One of its bugs is that it truncates file names at the
first embedded space.

> But I mostly disliked the name. Why are there two almost identical name
> for different functionality?

So annoying, and not limited to dired. I recently posted another in bug
report 43294 [1]. It's been languishing for ~6 weeks without a decision
for action, so your vote might be helpful there.

> I have seen also discussion about string-replace and replace-string, can
> we plase not? For the reason above. And for the new APIs added, please
> don't use naming patterns like word1-word2 and word2-word1 or similar
> where same words are used just with some slight variation.

+1

Also, match-string and string-match. IMO, match-string should be aliased
and to something like 'match-result' and deprecate the original name
with a long-horizon EOL to account for its ubiquity. @Lars: Should I
open a bug report separately for this?

[1] https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-09/msg00872.html

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-13 10:25       ` Boruch Baum
  2020-10-13 10:58         ` Arthur Miller
@ 2020-10-14  3:45         ` Lars Ingebrigtsen
  2020-10-14  4:19           ` Boruch Baum
  1 sibling, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2020-10-14  3:45 UTC (permalink / raw)
  To: Boruch Baum; +Cc: 43961, Richard Stallman

Boruch Baum <boruch_baum@gmx.com> writes:

>> Why not just have one call the other, and wrap the results in
>> abbreviate-file-name?
>
> You could, but you wouldn't be saving anything since the inner function
> would still need to perform the expansion, so for the abbreviated
> function you end up in effect with an inefficient (abbrev (expand file))
> instead of a choice between (abbrev file) or (expand file).

It's not about code efficiency -- whether this function is efficient or
not doesn't make any difference, since we'd deprecate it, and change the
callers.

> Also, much of the change ends up being defaliases, docstrings and
> deprecation notices, so its more clearly presented without nesting
> functions.

Sorry, I don't think this is a good change.  Renaming these functions
just because we have two almost identical ones (when we should just
remove one of them) doesn't make much sense.

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-14  3:45         ` Lars Ingebrigtsen
@ 2020-10-14  4:19           ` Boruch Baum
  0 siblings, 0 replies; 12+ messages in thread
From: Boruch Baum @ 2020-10-14  4:19 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 43961, Richard Stallman

On 2020-10-14 05:45, Lars Ingebrigtsen wrote:
> It's not about code efficiency -- whether this function is efficient or
> not doesn't make any difference, since we'd deprecate it, and change the
> callers.

> Sorry, I don't think this is a good change.  Renaming these functions
> just because we have two almost identical ones (when we should just
> remove one of them) doesn't make much sense.

Maybe you're catching me on a bad day, but I don't have the patience to
continue this discussion. IMO, you're making a glaringly bad call. If my
presentation was inadequate, that's on me; I can't present any clearer
than I have already done previously on this thread, so someone else is
going to need to carry the ball on this, or maybe you'll re-read the
thread more carefully and change your mind. I'm moving on to other
issues.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-13  4:08   ` Boruch Baum
  2020-10-13  4:54     ` Lars Ingebrigtsen
@ 2020-10-14  4:43     ` Richard Stallman
  2020-10-14  4:58       ` Lars Ingebrigtsen
  1 sibling, 1 reply; 12+ messages in thread
From: Richard Stallman @ 2020-10-14  4:43 UTC (permalink / raw)
  To: Boruch Baum; +Cc: 43961

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > As I think I mentioned, once (if) the patch is approved, a next step is
  > to grep the emacs core code-base and replace the function names.

I just did that.  dired-filename-at-point is not used anywhere except
dired-x.el.

I expect that none of the published Lisp packages uses
dired-filename-at-point.
-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)







^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-14  4:43     ` Richard Stallman
@ 2020-10-14  4:58       ` Lars Ingebrigtsen
  2020-10-15  3:57         ` Richard Stallman
  0 siblings, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2020-10-14  4:58 UTC (permalink / raw)
  To: Richard Stallman; +Cc: 43961, Boruch Baum

Richard Stallman <rms@gnu.org> writes:

>   > As I think I mentioned, once (if) the patch is approved, a next step is
>   > to grep the emacs core code-base and replace the function names.
>
> I just did that.  dired-filename-at-point is not used anywhere except
> dired-x.el.
>
> I expect that none of the published Lisp packages uses
> dired-filename-at-point.

And I looked at the code now, and it seems the bug reporter
misunderstood the point of the function: It's used to guess a file name
in non-dired buffers, while dired-file-name-at-point is a dired function
that just returns the file name in a dired buffer.

So they're totally unrelated functions, but dired-filename-at-point has
an unfortunate name.  I've now renamed it (but kept
dired-filename-at-point as an obsolete alias).

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point
  2020-10-14  4:58       ` Lars Ingebrigtsen
@ 2020-10-15  3:57         ` Richard Stallman
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Stallman @ 2020-10-15  3:57 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 43961, boruch_baum

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > So they're totally unrelated functions, but dired-filename-at-point has
  > an unfortunate name.  I've now renamed it (but kept
  > dired-filename-at-point as an obsolete alias).

Thanks for cleaning up that confusion.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)







^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-10-15  3:57 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-12 14:26 bug#43961: read carefully: dired-file-name-at-point vs dired-filename-at-point Boruch Baum
2020-10-13  3:49 ` Richard Stallman
2020-10-13  4:08   ` Boruch Baum
2020-10-13  4:54     ` Lars Ingebrigtsen
2020-10-13 10:25       ` Boruch Baum
2020-10-13 10:58         ` Arthur Miller
2020-10-13 11:27           ` Boruch Baum
2020-10-14  3:45         ` Lars Ingebrigtsen
2020-10-14  4:19           ` Boruch Baum
2020-10-14  4:43     ` Richard Stallman
2020-10-14  4:58       ` Lars Ingebrigtsen
2020-10-15  3:57         ` Richard Stallman

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).