* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly @ 2017-11-24 12:45 Michael Albinus 2017-11-24 13:37 ` Eli Zaretskii 2017-11-24 16:30 ` Drew Adams 0 siblings, 2 replies; 22+ messages in thread From: Michael Albinus @ 2017-11-24 12:45 UTC (permalink / raw) To: 29423 [-- Attachment #1: Type: text/plain, Size: 343 bytes --] Goto the *scratch* buffer, and perform M-: (ls-lisp-insert-directory "/tmp/" '(?F) nil nil nil) Move the cursor into the string /tmp/, and perform M-x describe-char There is no text property 'dired-filename, as it should. The following patch seems to cure the problem. Run the same test, you will see the text property 'dired-filename. [-- Attachment #2: Type: text/plain, Size: 504 bytes --] diff --git a/lisp/ls-lisp.el b/lisp/ls-lisp.el index caddc7f760..6765cc8dc9 100644 --- a/lisp/ls-lisp.el +++ b/lisp/ls-lisp.el @@ -841,9 +841,7 @@ ls-lisp-format " " (ls-lisp-format-time file-attr time-index) " " - (if (not (memq ?F switches)) ; ls-lisp-classify already did that - (propertize file-name 'dired-filename t) - file-name) + (propertize file-name 'dired-filename t) (if (stringp file-type) ; is a symbolic link (concat " -> " file-type)) "\n" [-- Attachment #3: Type: text/plain, Size: 7351 bytes --] This is a minor problem only. But I've stumbled over it, working on new Tramp tests. Any objection to install the patch? In GNU Emacs 27.0.50 (build 14, x86_64-pc-linux-gnu, GTK+ Version 3.22.24) of 2017-11-23 built on detlef Repository revision: 0092a856ff3900c3771408893fb7fd8d731de568 Windowing system distributor 'The X.Org Foundation', version 11.0.11905000 System Description: Ubuntu 17.10 Recent messages: Buffer B: Processing difference region 0 of 2 Processing difference regions ... done Refining difference region 1 ... Saving file /usr/local/src/emacs/lisp/ls-lisp.el... Wrote /usr/local/src/emacs/lisp/ls-lisp.el ls-lisp-format Continue... nil Type C-x 1 to delete the help window, C-M-v to scroll help. Mark set Configured using: 'configure --with-file-notification=inotify' Configured features: XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 LCMS2 Important settings: value of $LANG: en_US.UTF-8 value of $XMODIFIERS: @im=none locale-coding-system: utf-8 Major mode: Lisp Interaction Minor modes in effect: diff-auto-refine-mode: t erc-notify-mode: t erc-notifications-mode: t erc-list-mode: t erc-menu-mode: t erc-autojoin-mode: t erc-ring-mode: t erc-networks-mode: t erc-pcomplete-mode: t erc-track-mode: t erc-match-mode: t erc-button-mode: t erc-fill-mode: t erc-stamp-mode: t erc-netsplit-mode: t erc-irccontrols-mode: t erc-noncommands-mode: t erc-move-to-prompt-mode: t erc-readonly-mode: t url-handler-mode: t display-time-mode: t shell-dirtrack-mode: t icomplete-mode: t show-paren-mode: t tooltip-mode: t global-eldoc-mode: t eldoc-mode: t electric-indent-mode: t mouse-wheel-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t column-number-mode: t line-number-mode: t transient-mark-mode: t Load-path shadows: /home/albinus/src/elpa/packages/debbugs/debbugs-org hides /home/albinus/.emacs.d/elpa/debbugs-0.14/debbugs-org /home/albinus/src/elpa/packages/debbugs/debbugs-gnu hides /home/albinus/.emacs.d/elpa/debbugs-0.14/debbugs-gnu /home/albinus/src/elpa/packages/debbugs/debbugs hides /home/albinus/.emacs.d/elpa/debbugs-0.14/debbugs /home/albinus/src/elpa/packages/debbugs/debbugs-autoloads hides /home/albinus/.emacs.d/elpa/debbugs-0.14/debbugs-autoloads /home/albinus/src/elpa/packages/debbugs/debbugs-pkg hides /home/albinus/.emacs.d/elpa/debbugs-0.14/debbugs-pkg /home/albinus/src/elpa/packages/debbugs/debbugs-browse hides /home/albinus/.emacs.d/elpa/debbugs-0.14/debbugs-browse /home/albinus/.emacs.d/elpa/counsel-20171101.1121/counsel hides /home/albinus/.emacs.d/elpa/ivy-0.9.1/counsel /home/albinus/.emacs.d/elpa/swiper-20171105.42/swiper hides /home/albinus/.emacs.d/elpa/ivy-0.9.1/swiper /home/albinus/src/elpa/packages/tramp-theme/tramp-theme hides /home/albinus/.emacs.d/elpa/tramp-theme-0.2/tramp-theme /home/albinus/src/elpa/packages/tramp-theme/tramp-theme-autoloads hides /home/albinus/.emacs.d/elpa/tramp-theme-0.2/tramp-theme-autoloads /home/albinus/src/elpa/packages/tramp-theme/tramp-theme-pkg hides /home/albinus/.emacs.d/elpa/tramp-theme-0.2/tramp-theme-pkg /home/albinus/.emacs.d/elpa/telepathy-20131209.458/telepathy hides ~/lisp/telepathy ~/src/tramp/lisp/tramp-smb hides /usr/local/src/emacs/lisp/net/tramp-smb ~/src/tramp/lisp/tramp-uu hides /usr/local/src/emacs/lisp/net/tramp-uu ~/src/tramp/lisp/tramp-adb hides /usr/local/src/emacs/lisp/net/tramp-adb ~/src/tramp/lisp/tramp-cmds hides /usr/local/src/emacs/lisp/net/tramp-cmds ~/src/tramp/lisp/tramp-cache hides /usr/local/src/emacs/lisp/net/tramp-cache ~/src/tramp/lisp/trampver hides /usr/local/src/emacs/lisp/net/trampver ~/src/tramp/lisp/tramp-ftp hides /usr/local/src/emacs/lisp/net/tramp-ftp ~/src/tramp/lisp/tramp-sh hides /usr/local/src/emacs/lisp/net/tramp-sh ~/src/tramp/lisp/tramp hides /usr/local/src/emacs/lisp/net/tramp ~/src/tramp/lisp/tramp-loaddefs hides /usr/local/src/emacs/lisp/net/tramp-loaddefs ~/lisp/dbus hides /usr/local/src/emacs/lisp/net/dbus ~/src/tramp/lisp/tramp-gvfs hides /usr/local/src/emacs/lisp/net/tramp-gvfs ~/src/tramp/lisp/tramp-compat hides /usr/local/src/emacs/lisp/net/tramp-compat Features: (shadow sort mail-extr warnings emacsbug message rmc puny rfc822 mml mml-sec epa derived epg gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils add-log log-view pcvs-util ediff-vers ediff-merg ediff-wind ediff-diff ediff-mult ediff-help ediff-init ediff-util ediff erc-replace magit-utils crm cus-edit descr-text time-stamp misearch multi-isearch tramp-adb tramp-cmds tramp-ftp cl-print edebug eieio-opt speedbar sb-image ezimage dframe help-fns radix-tree ls-lisp files-x cl-extra help-mode tramp-archive-tests tramp-archive tramp-gvfs zeroconf url-util ert find-func ewoc debug vc-hg vc-git diff-mode easy-mmode bug-reference map cus-start cus-load elec-pair erc-notify erc-desktop-notifications notifications dbus xml erc-list erc-menu erc-join erc-ring erc-networks erc-pcomplete erc-track erc-match erc-button wid-edit erc-fill erc-stamp erc-netsplit erc-goodies erc erc-backend erc-compat thingatpt pp cperl-mode tramp-theme em-dirs esh-var esh-io esh-cmd esh-opt esh-ext esh-proc esh-arg esh-groups eshell esh-module esh-mode esh-util finder-inf rx docker-tramp tramp-cache slime-autoloads vagrant-tramp dash term disp-table ehelp info package easymenu epg-config url-handlers url-parse url-vars time tramp-sh tramp tramp-compat tramp-loaddefs trampver ucs-normalize shell pcomplete comint ansi-color ring parse-time format-spec advice auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs password-cache ido seq byte-opt gv bytecomp byte-compile cconv jka-compr icomplete paren vc cl-loaddefs cl-lib vc-dispatcher dired dired-loaddefs time-date mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite charscript charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs button faces cus-face macroexp files text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote dbusbind inotify lcms2 dynamic-setting system-font-setting font-render-setting move-toolbar gtk x-toolkit x multi-tty make-network-process emacs) Memory information: ((conses 16 591729 78592) (symbols 48 51583 5) (miscs 40 1187 1629) (strings 32 163059 9766) (string-bytes 1 4144809) (vectors 16 66157) (vector-slots 8 2194248 184812) (floats 8 143 706) (intervals 56 12315 1573) (buffers 992 44)) ^ permalink raw reply related [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 12:45 bug#29423: 27.0.50; ls-lisp does not handle -F switch properly Michael Albinus @ 2017-11-24 13:37 ` Eli Zaretskii 2017-11-24 13:41 ` Michael Albinus 2017-11-24 16:30 ` Drew Adams 1 sibling, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2017-11-24 13:37 UTC (permalink / raw) To: Michael Albinus; +Cc: 29423 > From: Michael Albinus <michael.albinus@gmx.de> > Date: Fri, 24 Nov 2017 13:45:45 +0100 > > Goto the *scratch* buffer, and perform > > M-: (ls-lisp-insert-directory "/tmp/" '(?F) nil nil nil) > > Move the cursor into the string /tmp/, and perform > > M-x describe-char > > There is no text property 'dired-filename, as it should. > > The following patch seems to cure the problem. Run the same test, you > will see the text property 'dired-filename. > > > [2:text/plain Hide] > > diff --git a/lisp/ls-lisp.el b/lisp/ls-lisp.el > index caddc7f760..6765cc8dc9 100644 > --- a/lisp/ls-lisp.el > +++ b/lisp/ls-lisp.el > @@ -841,9 +841,7 @@ ls-lisp-format > " " > (ls-lisp-format-time file-attr time-index) > " " > - (if (not (memq ?F switches)) ; ls-lisp-classify already did that > - (propertize file-name 'dired-filename t) > - file-name) > + (propertize file-name 'dired-filename t) > (if (stringp file-type) ; is a symbolic link > (concat " -> " file-type)) > "\n" How come ls-lisp-classify doesn't propertize the file name in this case? ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 13:37 ` Eli Zaretskii @ 2017-11-24 13:41 ` Michael Albinus 2017-11-24 14:56 ` Eli Zaretskii 2017-11-25 8:12 ` Eli Zaretskii 0 siblings, 2 replies; 22+ messages in thread From: Michael Albinus @ 2017-11-24 13:41 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 29423 Eli Zaretskii <eliz@gnu.org> writes: > How come ls-lisp-classify doesn't propertize the file name in this > case? Because it wasn't called. ls-lisp-classify-file was called only, if I'm not mistaken. Best regards, Michael. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 13:41 ` Michael Albinus @ 2017-11-24 14:56 ` Eli Zaretskii 2017-11-24 15:13 ` Michael Albinus 2017-11-25 8:12 ` Eli Zaretskii 1 sibling, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2017-11-24 14:56 UTC (permalink / raw) To: Michael Albinus; +Cc: 29423 > From: Michael Albinus <michael.albinus@gmx.de> > Cc: 29423@debbugs.gnu.org > Date: Fri, 24 Nov 2017 14:41:33 +0100 > > Eli Zaretskii <eliz@gnu.org> writes: > > > How come ls-lisp-classify doesn't propertize the file name in this > > case? > > Because it wasn't called. Then we should at least delete the propertize call from ls-lisp-classify, as part of your patch, no? ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 14:56 ` Eli Zaretskii @ 2017-11-24 15:13 ` Michael Albinus 2017-11-24 15:47 ` Eli Zaretskii 0 siblings, 1 reply; 22+ messages in thread From: Michael Albinus @ 2017-11-24 15:13 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 29423 Eli Zaretskii <eliz@gnu.org> writes: > Then we should at least delete the propertize call from > ls-lisp-classify, as part of your patch, no? Yes. At least my (new) Tramp tests still pass, after I've done this. Don't know, whether there are other insert-directory tests in Emacs' test subdirectory. Shall I commit to master? Best regards, Michael. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 15:13 ` Michael Albinus @ 2017-11-24 15:47 ` Eli Zaretskii 0 siblings, 0 replies; 22+ messages in thread From: Eli Zaretskii @ 2017-11-24 15:47 UTC (permalink / raw) To: Michael Albinus; +Cc: 29423 > From: Michael Albinus <michael.albinus@gmx.de> > Cc: 29423@debbugs.gnu.org > Date: Fri, 24 Nov 2017 16:13:51 +0100 > > Eli Zaretskii <eliz@gnu.org> writes: > > > Then we should at least delete the propertize call from > > ls-lisp-classify, as part of your patch, no? > > Yes. At least my (new) Tramp tests still pass, after I've done > this. Don't know, whether there are other insert-directory tests in > Emacs' test subdirectory. > > Shall I commit to master? Yes, please, and thanks. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 13:41 ` Michael Albinus 2017-11-24 14:56 ` Eli Zaretskii @ 2017-11-25 8:12 ` Eli Zaretskii 2017-11-25 8:59 ` Michael Albinus 1 sibling, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2017-11-25 8:12 UTC (permalink / raw) To: Michael Albinus; +Cc: 29423 > From: Michael Albinus <michael.albinus@gmx.de> > Cc: 29423@debbugs.gnu.org > Date: Fri, 24 Nov 2017 14:41:33 +0100 > > Eli Zaretskii <eliz@gnu.org> writes: > > > How come ls-lisp-classify doesn't propertize the file name in this > > case? > > Because it wasn't called. ls-lisp-classify-file was called only, if I'm > not mistaken. Right, and so the correct fix is below, I think. Do you agree? We can install this on the release branch, unless the original problem with Tramp exists only on master. Let me know. Thanks. diff --git a/lisp/ls-lisp.el b/lisp/ls-lisp.el index caddc7f..cf3bff5 100644 --- a/lisp/ls-lisp.el +++ b/lisp/ls-lisp.el @@ -713,23 +713,26 @@ ls-lisp-handle-switches (defun ls-lisp-classify-file (filename fattr) "Append a character to FILENAME indicating the file type. +This function puts the `dired-filename' property on FILENAME, but +not on the character indicator it appends. FATTR is the file attributes returned by `file-attributes' for the file. The file type indicators are `/' for directories, `@' for symbolic links, `|' for FIFOs, `=' for sockets, `*' for regular files that are executable, and nothing for other types of files." (let* ((type (car fattr)) (modestr (nth 8 fattr)) - (typestr (substring modestr 0 1))) + (typestr (substring modestr 0 1)) + (file-name (propertize filename 'dired-filename t))) (cond (type - (concat filename (if (eq type t) "/" "@"))) + (concat file-name (if (eq type t) "/" "@"))) ((string-match "x" modestr) - (concat filename "*")) + (concat file-name "*")) ((string= "p" typestr) - (concat filename "|")) + (concat file-name "|")) ((string= "s" typestr) - (concat filename "=")) - (t filename)))) + (concat file-name "=")) + (t file-name)))) (defun ls-lisp-classify (filedata) "Append a character to file name in FILEDATA indicating the file type. @@ -742,7 +745,6 @@ ls-lisp-classify are executable, and nothing for other types of files." (let ((file-name (car filedata)) (fattr (cdr filedata))) - (setq file-name (propertize file-name 'dired-filename t)) (cons (ls-lisp-classify-file file-name fattr) fattr))) (defun ls-lisp-extension (filename) ^ permalink raw reply related [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-25 8:12 ` Eli Zaretskii @ 2017-11-25 8:59 ` Michael Albinus 2017-11-25 10:37 ` Eli Zaretskii 0 siblings, 1 reply; 22+ messages in thread From: Michael Albinus @ 2017-11-25 8:59 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 29423 Eli Zaretskii <eliz@gnu.org> writes: Hi Eli, >> Because it wasn't called. ls-lisp-classify-file was called only, if I'm >> not mistaken. > > Right, and so the correct fix is below, I think. Do you agree? Yes. I've applied your patch instead of mine, and the corresponding test still passes. > We can install this on the release branch, unless the original problem > with Tramp exists only on master. Let me know. Yes, please. The problem does not happen in existing Tramp, but in the not published yet tramp-archive.el I'm working on. It shall support as many older Emacsen as the existing Tramp; if it would be fixed in Emacs 26 it would be great. If needed, I will add some compat code for older Emacsen (24, 25), as it is tradition in Tramp. > Thanks. Best regards, Michael. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-25 8:59 ` Michael Albinus @ 2017-11-25 10:37 ` Eli Zaretskii 0 siblings, 0 replies; 22+ messages in thread From: Eli Zaretskii @ 2017-11-25 10:37 UTC (permalink / raw) To: Michael Albinus; +Cc: 29423-done > From: Michael Albinus <michael.albinus@gmx.de> > Cc: 29423@debbugs.gnu.org > Date: Sat, 25 Nov 2017 09:59:52 +0100 > > > We can install this on the release branch, unless the original problem > > with Tramp exists only on master. Let me know. > > Yes, please. The problem does not happen in existing Tramp, but in the > not published yet tramp-archive.el I'm working on. It shall support as > many older Emacsen as the existing Tramp; if it would be fixed in Emacs > 26 it would be great. Done. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 12:45 bug#29423: 27.0.50; ls-lisp does not handle -F switch properly Michael Albinus 2017-11-24 13:37 ` Eli Zaretskii @ 2017-11-24 16:30 ` Drew Adams 2017-11-24 16:56 ` Michael Albinus 2017-11-24 17:02 ` Eli Zaretskii 1 sibling, 2 replies; 22+ messages in thread From: Drew Adams @ 2017-11-24 16:30 UTC (permalink / raw) To: Michael Albinus, 29423 > Goto the *scratch* buffer, and perform > M-: (ls-lisp-insert-directory "/tmp/" '(?F) nil nil nil) > Move the cursor into the string /tmp/, and perform > M-x describe-char > > There is no text property 'dired-filename, as it should. I think you're talking (only) about the final / char. That seems to be the only place where the property is not present. Is that / part of the (directory as) file name? Dunno whether that consideration helps here - probably not. What to cover by the property really depends on what the property is used for. Unfortunately perhaps, unlike the case for functions and variables, there is no doc string for text properties. Unless something is called out for this in some doc string or in code comments, only the current uses of the property can guide what it should apply to. I don't know whether the / should have that property. I have checked and see that in Emacs 22 it has it, and thereafter it does not. Regression? Intentional change? The current uses of the property, as I quickly check them don't suggest that it matters whether / has the property. Do you have something (e.g. some use case) particular in mind, where you think that the / should have the property? Should we consider code that expects the result of checking for that property to give the same position regardless of whether switch `F' is used? Should the file name be considered to be the same, regardless of whether a / is appended? In sum, is this a bug to be fixed, a design question, or design that was already changed intentionally for Emacs 23? (I have no idea, and no code of mine depends on what is decided, AFAIK.) ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 16:30 ` Drew Adams @ 2017-11-24 16:56 ` Michael Albinus 2017-11-24 17:12 ` Drew Adams 2017-11-24 17:02 ` Eli Zaretskii 1 sibling, 1 reply; 22+ messages in thread From: Michael Albinus @ 2017-11-24 16:56 UTC (permalink / raw) To: Drew Adams; +Cc: 29423 Drew Adams <drew.adams@oracle.com> writes: Hi Drew, >> Goto the *scratch* buffer, and perform >> M-: (ls-lisp-insert-directory "/tmp/" '(?F) nil nil nil) >> Move the cursor into the string /tmp/, and perform >> M-x describe-char >> >> There is no text property 'dired-filename, as it should. > > I think you're talking (only) about the final / char. > That seems to be the only place where the property is not > present. Yes. Finally, it was triggered by Tramp using the F switch. This was added by Tramp in order to get a trailing / for directory names in the directory listing. > Is that / part of the (directory as) file name? Dunno > whether that consideration helps here - probably not. > What to cover by the property really depends on what the > property is used for. No, it is not part of the file name. Like the trailing " -> foo" for symlinks. My problem is, that the whole file name is missing the text property, not only the trailing slash. > Unfortunately perhaps, unlike the case for functions and > variables, there is no doc string for text properties. > Unless something is called out for this in some doc string > or in code comments, only the current uses of the property > can guide what it should apply to. I agree. However, in the given case, the text property 'dired-filename shall highlight exactly the file name. It is used later in dired (as the name of the property says), but it is also used in Tramp, because for some of the Tramp backends, ls-lisp-insert-directory is used internally, and Tramp needs some massage on the result. > I don't know whether the / should have that property. No. > I have checked and see that in Emacs 22 it has it, and > thereafter it does not. Regression? Intentional change? Likely intentionally. > The current uses of the property, as I quickly check them > don't suggest that it matters whether / has the property. > > Do you have something (e.g. some use case) particular in > mind, where you think that the / should have the property? Again, this is not my point. My point is, that the name itself does not have the property. > Should we consider code that expects the result of checking > for that property to give the same position regardless of > whether switch `F' is used? Should the file name be > considered to be the same, regardless of whether a / is > appended? Parsing the output of any insert-directory is a pain. Therefore, it is helpful to know that the file name part of this output is marked with 'dired-filename. Even in backends where Tramp does not use ls-lisp-insert-directory, Tramp tries its best to set this text property for dired. See tramp-sh-handle-insert-directory, for example. > In sum, is this a bug to be fixed, a design question, or > design that was already changed intentionally for Emacs 23? > > (I have no idea, and no code of mine depends on what is > decided, AFAIK.) I believe it is a bug. I have just applied my patch locally, and I have rerun the whole Emacs testsuite. Several errors did appear I haven't seen before. I'll check, whether they are related to my change. Best regards, Michael. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 16:56 ` Michael Albinus @ 2017-11-24 17:12 ` Drew Adams 2017-11-24 18:48 ` Michael Albinus 0 siblings, 1 reply; 22+ messages in thread From: Drew Adams @ 2017-11-24 17:12 UTC (permalink / raw) To: Michael Albinus; +Cc: 29423 > > I think you're talking (only) about the final / char. > > That seems to be the only place where the property is not > > present. > > Yes. Finally, it was triggered by Tramp using the F switch. This was > added by Tramp in order to get a trailing / for directory names in the > directory listing. You said "Yes", but below you seem to say "No". Is the / the only place where the property is not present? > > Is that / part of the (directory as) file name? Dunno > > whether that consideration helps here - probably not. > > What to cover by the property really depends on what the > > property is used for. > > No, it is not part of the file name. Like the trailing " -> foo" for > symlinks. > > My problem is, that the whole file name is missing the text property, > not only the trailing slash. (See above for (my) confusion about this.) That's not what I see, in any Emacs release or in the 26.1 prerelease (MS Windows binary). Perhaps what you see is a problem introduced after that prerelease or is platform-dependent? > > Unfortunately perhaps, unlike the case for functions and > > variables, there is no doc string for text properties. > > Unless something is called out for this in some doc string > > or in code comments, only the current uses of the property > > can guide what it should apply to. > > I agree. However, in the given case, the text property 'dired-filename > shall highlight exactly the file name. It is used later in dired (as the > name of the property says), but it is also used in Tramp, because for > some of the Tramp backends, ls-lisp-insert-directory is used internally, > and Tramp needs some massage on the result. I definitely agree that the name of the directory (sans /) needs the property. If that's missing then there is likely a regression. > Parsing the output of any insert-directory is a pain. Therefore, it is > helpful to know that the file name part of this output is marked with > 'dired-filename. I agree 100% that the name needs the property. As the only char I see missing the property is the final /, I thought that's what you were asking about and reporting as a problem. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 17:12 ` Drew Adams @ 2017-11-24 18:48 ` Michael Albinus 2017-11-24 19:28 ` Drew Adams 0 siblings, 1 reply; 22+ messages in thread From: Michael Albinus @ 2017-11-24 18:48 UTC (permalink / raw) To: Drew Adams; +Cc: 29423 Drew Adams <drew.adams@oracle.com> writes: Hi Drew, > That's not what I see, in any Emacs release or in the > 26.1 prerelease (MS Windows binary). Perhaps what you > see is a problem introduced after that prerelease or > is platform-dependent? I've just checked again my recipe with the Emacs 26 branch, started as "emacs -Q". Reproduced. And I could reproduce it also with Emacs 25, as provided by Ubuntu 17.10. Maybe the difference is that I haven't said explicitly that you need (require 'ls-lisp) prior my recipe. I thought it was obvious, due to the subject of the bug report. Sorry. > As the only char I see missing the property is the > final /, I thought that's what you were asking about > and reporting as a problem. Use ls-lisp. Best regards, Michael. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 18:48 ` Michael Albinus @ 2017-11-24 19:28 ` Drew Adams 2017-11-24 19:51 ` Noam Postavsky 0 siblings, 1 reply; 22+ messages in thread From: Drew Adams @ 2017-11-24 19:28 UTC (permalink / raw) To: Michael Albinus; +Cc: 29423 > > That's not what I see, in any Emacs release or in the > > 26.1 prerelease (MS Windows binary). Perhaps what you > > see is a problem introduced after that prerelease or > > is platform-dependent? > > I've just checked again my recipe with the Emacs 26 branch, started as > "emacs -Q". Reproduced. And I could reproduce it also with Emacs 25, as > provided by Ubuntu 17.10. > > Maybe the difference is that I haven't said explicitly that you need > (require 'ls-lisp) prior my recipe. I thought it was obvious, due to the > subject of the bug report. Sorry. > > > As the only char I see missing the property is the > > final /, I thought that's what you were asking about > > and reporting as a problem. > > Use ls-lisp. I did it again, from emacs -Q, with the Emacs 26.1 pretest. I tried with M-x load-library ls-lisp.el, and I tried with M-x load-library ls-lisp.elc. And I think that neither should be needed, since Emacs on MS Windows (which I'm using) uses ls-lisp by default. I still see what I reported earlier: the property is on the directory name (but not on the /). I'm guessing that the Emacs 26 you're using is something later than the pretest. Or else the difference has something to do with the platform. HTH. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 19:28 ` Drew Adams @ 2017-11-24 19:51 ` Noam Postavsky 2017-11-24 20:27 ` Drew Adams 0 siblings, 1 reply; 22+ messages in thread From: Noam Postavsky @ 2017-11-24 19:51 UTC (permalink / raw) To: Drew Adams; +Cc: Michael Albinus, 29423 On Fri, Nov 24, 2017 at 2:28 PM, Drew Adams <drew.adams@oracle.com> wrote: > I did it again, from emacs -Q, with the Emacs 26.1 pretest. > > I tried with M-x load-library ls-lisp.el, and > I tried with M-x load-library ls-lisp.elc. And I > think that neither should be needed, since Emacs > on MS Windows (which I'm using) uses ls-lisp by > default. > > I still see what I reported earlier: the property > is on the directory name (but not on the /). I can reproduce according to Michael's instructions on MS-Windows, in Emacs 24.5, 25.3, and an Emacs 26 pretest (I don't have exactly 26.0.90 handy though). > I'm guessing that the Emacs 26 you're using is > something later than the pretest. Or else the > difference has something to do with the platform. Are you looking at a dired buffer? That's the only I can replicate what you are reporting. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 19:51 ` Noam Postavsky @ 2017-11-24 20:27 ` Drew Adams 2017-11-24 20:37 ` Noam Postavsky 0 siblings, 1 reply; 22+ messages in thread From: Drew Adams @ 2017-11-24 20:27 UTC (permalink / raw) To: Noam Postavsky; +Cc: Michael Albinus, 29423 > > I did it again, from emacs -Q, with the Emacs 26.1 pretest. > > > > I tried with M-x load-library ls-lisp.el, and > > I tried with M-x load-library ls-lisp.elc. And I > > think that neither should be needed, since Emacs > > on MS Windows (which I'm using) uses ls-lisp by > > default. > > > > I still see what I reported earlier: the property > > is on the directory name (but not on the /). > > I can reproduce according to Michael's instructions on MS-Windows, in > Emacs 24.5, 25.3, and an Emacs 26 pretest (I don't have exactly > 26.0.90 handy though). OK. If you can repro it then I'm probably not helping here. > > I'm guessing that the Emacs 26 you're using is > > something later than the pretest. Or else the > > difference has something to do with the platform. > > Are you looking at a dired buffer? That's the only I can replicate > what you are reporting. I can't parse your last sentence. Yes, I was looking at a dired buffer. I tested with this pretest build: In GNU Emacs 26.0.90 (build 3, x86_64-w64-mingw32) of 2017-10-13 built on LAPHROAIG Repository revision: 906224eba147bdfc0514090064e8e8f53160f1d4 Windowing system distributor 'Microsoft Corp.', version 6.1.7601 I just repeated the test with this other pretest build, and I see the same things as before. In GNU Emacs 26.0.90 (build 3, x86_64-w64-mingw32) of 2017-10-13 built on LAPHROAIG Repository revision: 906224eba147bdfc0514090064e8e8f53160f1d4 Windowing system distributor 'Microsoft Corp.', version 6.1.7601 And I just repeated the test with Emacs 25.3.1, and I see the same things I reported earlier. In GNU Emacs 25.3.1 (x86_64-w64-mingw32) of 2017-09-26 built on LAPHROAIG Windowing system distributor 'Microsoft Corp.', version 6.1.7601 Configured using: 'configure --without-dbus --without-compress-install 'CFLAGS=-O2 -static -g3' PKG_CONFIG_PATH=/mingw64/lib/pkgconfig' HTH. (I'm still guessing that you guys are using a more recent Emacs 26 than these pretests, and that something broke this since these pretest builds.) ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 20:27 ` Drew Adams @ 2017-11-24 20:37 ` Noam Postavsky 2017-11-24 20:51 ` Drew Adams 0 siblings, 1 reply; 22+ messages in thread From: Noam Postavsky @ 2017-11-24 20:37 UTC (permalink / raw) To: Drew Adams; +Cc: Michael Albinus, 29423 On Fri, Nov 24, 2017 at 3:27 PM, Drew Adams <drew.adams@oracle.com> wrote: >> Are you looking at a dired buffer? That's the only I can replicate ^ way >> what you are reporting. > Yes, I was looking > at a dired buffer. Then you didn't follow the instructions in the OP: Goto the *scratch* buffer, and perform M-: (ls-lisp-insert-directory "/tmp/" '(?F) nil nil nil) No calls to dired. The ls-lisp-insert-directory function does not create a dired buffer. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 20:37 ` Noam Postavsky @ 2017-11-24 20:51 ` Drew Adams 0 siblings, 0 replies; 22+ messages in thread From: Drew Adams @ 2017-11-24 20:51 UTC (permalink / raw) To: Noam Postavsky; +Cc: Michael Albinus, 29423 > > Yes, I was looking at a dired buffer. > > Then you didn't follow the instructions in the OP: > Goto the *scratch* buffer, and perform > M-: (ls-lisp-insert-directory "/tmp/" '(?F) nil nil nil) > > No calls to dired. The ls-lisp-insert-directory function does not > create a dired buffer. I see, and I see what you see if I follow the recipe. I missed that. Sorry for the noise. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 16:30 ` Drew Adams 2017-11-24 16:56 ` Michael Albinus @ 2017-11-24 17:02 ` Eli Zaretskii 2017-11-24 18:49 ` Michael Albinus 1 sibling, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2017-11-24 17:02 UTC (permalink / raw) To: Drew Adams; +Cc: michael.albinus, 29423 > Date: Fri, 24 Nov 2017 08:30:47 -0800 (PST) > From: Drew Adams <drew.adams@oracle.com> > > > Goto the *scratch* buffer, and perform > > M-: (ls-lisp-insert-directory "/tmp/" '(?F) nil nil nil) > > Move the cursor into the string /tmp/, and perform > > M-x describe-char > > > > There is no text property 'dired-filename, as it should. > > I think you're talking (only) about the final / char. > That seems to be the only place where the property is not > present. > > Is that / part of the (directory as) file name? No, it isn't. It's what -F append to the file name to indicate that it's a directory. So if that's the problem, the patch is not TRT, IMO. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 17:02 ` Eli Zaretskii @ 2017-11-24 18:49 ` Michael Albinus 2017-11-24 19:52 ` Eli Zaretskii 0 siblings, 1 reply; 22+ messages in thread From: Michael Albinus @ 2017-11-24 18:49 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 29423 Eli Zaretskii <eliz@gnu.org> writes: >> Is that / part of the (directory as) file name? > > No, it isn't. It's what -F append to the file name to indicate that > it's a directory. > > So if that's the problem, the patch is not TRT, IMO. I'll check. Best regards, Michael. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 18:49 ` Michael Albinus @ 2017-11-24 19:52 ` Eli Zaretskii 2017-11-24 21:03 ` Michael Albinus 0 siblings, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2017-11-24 19:52 UTC (permalink / raw) To: Michael Albinus; +Cc: 29423 > From: Michael Albinus <michael.albinus@gmx.de> > Cc: Drew Adams <drew.adams@oracle.com>, 29423@debbugs.gnu.org > Date: Fri, 24 Nov 2017 19:49:56 +0100 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> Is that / part of the (directory as) file name? > > > > No, it isn't. It's what -F append to the file name to indicate that > > it's a directory. > > > > So if that's the problem, the patch is not TRT, IMO. > > I'll check. The more I think about this the less I understand how your patch is going to work. How can ls-lisp-format know what was appended to a file name by ls-lisp-classify-file? ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#29423: 27.0.50; ls-lisp does not handle -F switch properly 2017-11-24 19:52 ` Eli Zaretskii @ 2017-11-24 21:03 ` Michael Albinus 0 siblings, 0 replies; 22+ messages in thread From: Michael Albinus @ 2017-11-24 21:03 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 29423 Eli Zaretskii <eliz@gnu.org> writes: > The more I think about this the less I understand how your patch is > going to work. Me too. > How can ls-lisp-format know what was appended to a file name by > ls-lisp-classify-file? Give me more time to check, please. Best regards, Michael. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-11-25 10:37 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-11-24 12:45 bug#29423: 27.0.50; ls-lisp does not handle -F switch properly Michael Albinus 2017-11-24 13:37 ` Eli Zaretskii 2017-11-24 13:41 ` Michael Albinus 2017-11-24 14:56 ` Eli Zaretskii 2017-11-24 15:13 ` Michael Albinus 2017-11-24 15:47 ` Eli Zaretskii 2017-11-25 8:12 ` Eli Zaretskii 2017-11-25 8:59 ` Michael Albinus 2017-11-25 10:37 ` Eli Zaretskii 2017-11-24 16:30 ` Drew Adams 2017-11-24 16:56 ` Michael Albinus 2017-11-24 17:12 ` Drew Adams 2017-11-24 18:48 ` Michael Albinus 2017-11-24 19:28 ` Drew Adams 2017-11-24 19:51 ` Noam Postavsky 2017-11-24 20:27 ` Drew Adams 2017-11-24 20:37 ` Noam Postavsky 2017-11-24 20:51 ` Drew Adams 2017-11-24 17:02 ` Eli Zaretskii 2017-11-24 18:49 ` Michael Albinus 2017-11-24 19:52 ` Eli Zaretskii 2017-11-24 21:03 ` Michael Albinus
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.