* bug#4819: file-truename's undocumented behavior @ 2009-10-28 0:28 MON KEY 2009-10-28 1:52 ` Stefan Monnier 0 siblings, 1 reply; 7+ messages in thread From: MON KEY @ 2009-10-28 0:28 UTC (permalink / raw) To: bug-gnu-emacs This form returns a value. (file-truename "") WHY? I just spent 2 1/2 hours in a break-loop three functions away trying to debug _undocumented_ behavior. GNU Emacs 23.1.50.1 (i386-mingw-nt5.1.2600) of 2009-06-30 on LENNART-69DE564 (patched) ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#4819: file-truename's undocumented behavior 2009-10-28 0:28 bug#4819: file-truename's undocumented behavior MON KEY @ 2009-10-28 1:52 ` Stefan Monnier 2009-10-28 20:01 ` MON KEY 0 siblings, 1 reply; 7+ messages in thread From: Stefan Monnier @ 2009-10-28 1:52 UTC (permalink / raw) To: MON KEY; +Cc: 4819, bug-gnu-emacs > This form returns a value. > (file-truename "") > WHY? Why not? What value did you expect? > I just spent 2 1/2 hours in a break-loop three functions away trying > to debug _undocumented_ behavior. Which part of the documentation do you think this behavior contradicts? Stefan ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#4819: file-truename's undocumented behavior 2009-10-28 1:52 ` Stefan Monnier @ 2009-10-28 20:01 ` MON KEY 2009-10-29 0:37 ` Stefan Monnier 0 siblings, 1 reply; 7+ messages in thread From: MON KEY @ 2009-10-28 20:01 UTC (permalink / raw) To: Stefan Monnier; +Cc: 4819, bug-gnu-emacs [-- Attachment #1: Type: text/plain, Size: 5480 bytes --] On Tue, Oct 27, 2009 at 9:52 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote: >> This form returns a value. >> (file-truename "") > >> WHY? > > Why not? What value did you expect? I assumed nil. I'm not sure what I would expect esp. having since noted that: (file-relative-name "") ;=> "." (file-exists-p "") ;=> t (file-readable-p "") ;=> t (file-directory-p "") ;=> t (file-name-as-directory "") ;=> "./" However, discarding the state of my "least surprisedness", I would _not_ expect that file-truename would step all over the match-data, and where it does so not before first: a) Using string-match-p where applicable; b) Noting that it does so in the documention. i.e. as per `split-string'. > >> I just spent 2 1/2 hours in a break-loop three functions away trying >> to debug _undocumented_ behavior. > > Which part of the documentation do you think this behavior contradicts? > This part: (file-name-absolute-p "") ;=> nil (file-symlink-p "") ;=> nil On which systems/platforms does "" denote an absolute filename? On which systems/platforms does "" denote a symbolic link for a filename? ,---- (documentation 'file-truename) | Return the truename of filename, which should be absolute. | The truename of a file name is found by chasing symbolic links | both at the level of the file and at the level of the directories | containing it, until no links are left at any level. `---- And, this other part where documentation: a) Neglects to mention that this function invokes repeatedly invokes `string-match' twice per invokation. {...} (unless (string-match "[[*?]" filename) (string-match "~[^/]*/?" filename)) {...} {...} ((and (string= (substring filename 0 1) "~") (string-match "~[^/]*/?" filename)) {...} b) Neglects to mention that the remaining args COUNTER and PREV-DIRS are iterative counters for operations on recursive calls. Which means that where file-truename recurses `string-match' may be invoked more than twice times. ,---- Comments in the definition of `file-truename' in files.el |;; Don't document the optional arguments. |;; COUNTER and PREV-DIRS are only used in recursive calls. |;; COUNTER can be a cons cell whose car is the count of how many |;; more links to chase before getting an error. |;; PREV-DIRS can be a cons cell whose car is an alist |;; of truenames we've just recently computed. `---- It was only as a late afterthought that I realized that it wasn't _MY-CODE+ clobbering the match-data - as is usual : ) but maybe Emacs'. The realization was elusive because the recursion only happens when the w32 conditional which drops into a recursion predicated on the return value of `w32-long-file-name'. {...} (setq filename (concat (file-truename rest) missing)) {...} Why not mention in the docs that on w32 `w32-long-file-name' may be a more suitable alternative esp. as it is a primitive and as it will expand "8.3 DOS" short name aliases in the process. (Again, per _existing_ comments in body of `file-truename's definition). I understand _why_ the optional args have been left undocumented - they are essentially hacks which the user shouldn't rely on. However, I don't understand _why_ the w32 hack isn't made known esp. where the hack is applicable for user code and is indicated as the preferred solution... W/re the string-match vs. string-matchp doesn't the following accomplish the same: ;;; ============================== *** /files.el-p 2009-10-28 15:49:38.843750000 -0400 --- /emacs/lisp/files.el 2009-06-30 15:51:22.000000000 -0400 *************** *** 893,904 **** (setq filename (expand-file-name filename)) (if (string= filename "") (setq filename "/"))) ! ((and (string= (substring filename 0 1) "~") ! (string-match-p "~[^/]*/?" filename)) ! (string-match "~[^/]*/?" filename) ! (let ((first-part ! (substring filename 0 (match-end 0))) ! (rest (substring filename (match-end 0)))) (setq filename (concat (expand-file-name first-part) rest))))) (or counter (setq counter (list 100))) --- 893,903 ---- (setq filename (expand-file-name filename)) (if (string= filename "") (setq filename "/"))) ! ((and (string= (substring filename 0 1) "~") ! (string-match "~[^/]*/?" filename)) ! (let ((first-part ! (substring filename 0 (match-end 0))) ! (rest (substring filename (match-end 0)))) (setq filename (concat (expand-file-name first-part) rest))))) (or counter (setq counter (list 100))) *************** *** 930,936 **** (if handler (setq filename (funcall handler 'file-truename filename)) ;; If filename contains a wildcard, newname will be the old name. ! (unless (string-match-p "[[*?]" filename) ;; If filename exists, use the long name. If it doesn't exist, ;; drill down until we find a directory that exists, and use ;; the long name of that, with the extra non-existent path --- 929,935 ---- (if handler (setq filename (funcall handler 'file-truename filename)) ;; If filename contains a wildcard, newname will be the old name. ! (unless (string-match "[[*?]" filename) ;; If filename exists, use the long name. If it doesn't exist, ;; drill down until we find a directory that exists, and use ;; the long name of that, with the extra non-existent path [-- Attachment #2: diff.files.el --] [-- Type: application/octet-stream, Size: 2002 bytes --] *** /files.el-p 2009-10-28 15:49:38.843750000 -0400 --- /emacs/lisp/files.el 2009-06-30 15:51:22.000000000 -0400 *************** *** 893,904 **** (setq filename (expand-file-name filename)) (if (string= filename "") (setq filename "/"))) ! ((and (string= (substring filename 0 1) "~") ! (string-match-p "~[^/]*/?" filename)) ! (string-match "~[^/]*/?" filename) ! (let ((first-part ! (substring filename 0 (match-end 0))) ! (rest (substring filename (match-end 0)))) (setq filename (concat (expand-file-name first-part) rest))))) (or counter (setq counter (list 100))) --- 893,903 ---- (setq filename (expand-file-name filename)) (if (string= filename "") (setq filename "/"))) ! ((and (string= (substring filename 0 1) "~") ! (string-match "~[^/]*/?" filename)) ! (let ((first-part ! (substring filename 0 (match-end 0))) ! (rest (substring filename (match-end 0)))) (setq filename (concat (expand-file-name first-part) rest))))) (or counter (setq counter (list 100))) *************** *** 930,936 **** (if handler (setq filename (funcall handler 'file-truename filename)) ;; If filename contains a wildcard, newname will be the old name. ! (unless (string-match-p "[[*?]" filename) ;; If filename exists, use the long name. If it doesn't exist, ;; drill down until we find a directory that exists, and use ;; the long name of that, with the extra non-existent path --- 929,935 ---- (if handler (setq filename (funcall handler 'file-truename filename)) ;; If filename contains a wildcard, newname will be the old name. ! (unless (string-match "[[*?]" filename) ;; If filename exists, use the long name. If it doesn't exist, ;; drill down until we find a directory that exists, and use ;; the long name of that, with the extra non-existent path ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#4819: file-truename's undocumented behavior 2009-10-28 20:01 ` MON KEY @ 2009-10-29 0:37 ` Stefan Monnier 2009-10-29 0:56 ` Lennart Borgman ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Stefan Monnier @ 2009-10-29 0:37 UTC (permalink / raw) To: MON KEY; +Cc: 4819, bug-gnu-emacs > I assumed nil. That would be very unexpected, since file-truename otherwise always returns a string when passed a string. > However, discarding the state of my "least surprisedness", I would > _not_ expect that file-truename would step all over the match-data, > and where it does so not before first: Most functions don't preserve the match-data, and even more so for functions that manipulate strings or buffers. > b) Noting that it does so in the documention. i.e. as per `split-string'. It's the other way around: the few functions that preserve the match-data should be documented as such (better yet: the byte-compiler should be taught about them, so it can detect when we use the match-data after it got clobbered). >>> I just spent 2 1/2 hours in a break-loop three functions away trying >>> to debug _undocumented_ behavior. >> Which part of the documentation do you think this behavior contradicts? > This part: > (file-name-absolute-p "") ;=> nil > (file-symlink-p "") ;=> nil That's not a part of the documentation. > Why not mention in the docs that on w32 `w32-long-file-name' may be > a more suitable alternative esp. as it is a primitive and as it will > expand "8.3 DOS" short name aliases in the process. (Again, per > _existing_ comments in body of `file-truename's definition). Elisp should generally not be w32-specific, so ratehr than use w32-long-file-name we should maybe change file-truename correspondingly. That doesn't mean I think it's the right thing to do: I know next to nothing about this issue. > ! ((and (string= (substring filename 0 1) "~") > ! (string-match-p "~[^/]*/?" filename)) > ! (string-match "~[^/]*/?" filename) > ! (let ((first-part > ! (substring filename 0 (match-end 0))) > ! (rest (substring filename (match-end 0)))) What's the point? If you're going to use string-match in the end, you might as well do it right away. Stefan ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#4819: file-truename's undocumented behavior 2009-10-29 0:37 ` Stefan Monnier @ 2009-10-29 0:56 ` Lennart Borgman 2009-11-07 0:01 ` MON KEY 2011-07-09 5:51 ` Glenn Morris 2 siblings, 0 replies; 7+ messages in thread From: Lennart Borgman @ 2009-10-29 0:56 UTC (permalink / raw) To: Stefan Monnier, 4819; +Cc: bug-gnu-emacs, MON KEY On Thu, Oct 29, 2009 at 1:37 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote: > >> Why not mention in the docs that on w32 `w32-long-file-name' may be >> a more suitable alternative esp. as it is a primitive and as it will >> expand "8.3 DOS" short name aliases in the process. (Again, per >> _existing_ comments in body of `file-truename's definition). > > Elisp should generally not be w32-specific, so ratehr than use > w32-long-file-name we should maybe change > file-truename correspondingly. That doesn't mean I think it's the right > thing to do: I know next to nothing about this issue. It already calls w32-long-file-name. (I am not sure what MON means.) ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#4819: file-truename's undocumented behavior 2009-10-29 0:37 ` Stefan Monnier 2009-10-29 0:56 ` Lennart Borgman @ 2009-11-07 0:01 ` MON KEY 2011-07-09 5:51 ` Glenn Morris 2 siblings, 0 replies; 7+ messages in thread From: MON KEY @ 2009-11-07 0:01 UTC (permalink / raw) To: Stefan Monnier; +Cc: 4819, bug-gnu-emacs Stefan, I'm very sorry for the delayed response things have been hectic of late. On Wed, Oct 28, 2009 at 7:37 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote: >> I assumed nil. > > That would be very unexpected, since file-truename otherwise always > returns a string when passed a string. Yes, understood; most likely my assumptions are predicated by these seemingly contradictory behaviors: (file-name-absolute-p "") ;=> nil (file-symlink-p "") ;=> nil > > It's the other way around: the few functions that preserve the > match-data should be documented as such (better yet: the byte-compiler > should be taught about them, so it can detect when we use the > match-data after it got clobbered). No argument there :) >>> Which part of the documentation do you think this behavior contradicts? > >> This part: >> (file-name-absolute-p "") ;=> nil >> (file-symlink-p "") ;=> nil > > That's not a part of the documentation. You're right. None the less, this behaviour does contradict behaviour indicated by said docs. > Elisp should generally not be w32-specific, so ratehr than use > w32-long-file-name we should maybe change > file-truename correspondingly. That doesn't mean I think it's the right > thing to do: I know next to nothing about this issue. Best I can gather the existing w32 conditional branch has been around for a long time. i.e. the email address in the comments carry a {...}@harlequin.co.uk domain. > >> ! ((and (string= (substring filename 0 1) "~") >> ! (string-match-p "~[^/]*/?" filename)) >> ! (string-match "~[^/]*/?" filename) >> ! (let ((first-part >> ! (substring filename 0 (match-end 0))) >> ! (rest (substring filename (match-end 0)))) > > What's the point? To avoid setting the match-data b/c it _is not_ necessarily going to be used per the conditional. > If you're going to use string-match in the end, you might as well do it right > away. This is wrong. Though, it may explain how/why the existing situation persists :) > Stefan > s_P ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#4819: file-truename's undocumented behavior 2009-10-29 0:37 ` Stefan Monnier 2009-10-29 0:56 ` Lennart Borgman 2009-11-07 0:01 ` MON KEY @ 2011-07-09 5:51 ` Glenn Morris 2 siblings, 0 replies; 7+ messages in thread From: Glenn Morris @ 2011-07-09 5:51 UTC (permalink / raw) To: 4819-done I don't see a need to keep open this particular report, which was marked "wontfix" some time ago. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-07-09 5:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-10-28 0:28 bug#4819: file-truename's undocumented behavior MON KEY 2009-10-28 1:52 ` Stefan Monnier 2009-10-28 20:01 ` MON KEY 2009-10-29 0:37 ` Stefan Monnier 2009-10-29 0:56 ` Lennart Borgman 2009-11-07 0:01 ` MON KEY 2011-07-09 5:51 ` Glenn Morris
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).