all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#63142: 30.0.50; QUOTING_STYLE can break dired
@ 2023-04-28  9:35 Yuri D'Elia
  2023-04-28 10:27 ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Yuri D'Elia @ 2023-04-28  9:35 UTC (permalink / raw)
  To: 63142


If the environment variable QUOTING_STYLE is set, GNU "ls" can return a
format that dired doesn't like.

QUOTING_STYLE=shell will quote files with spaces such as 'a b', and
trying to interact with these files with dired results in:

dired-get-file-for-visit: File no longer exists; type g to update Dired buffer

Interestingly, even using "ls --dired" does the same if QUOTING_STYLE is
set.

Should dired handle QUOTING_STYLE (looking at the coreutils manual it
seems that other tools besides ls handle it) or should dired actively
suppress the quoting mode?






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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-28  9:35 bug#63142: 30.0.50; QUOTING_STYLE can break dired Yuri D'Elia
@ 2023-04-28 10:27 ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-04-28 10:28 ` Eli Zaretskii
  2023-05-05 11:04 ` Mattias Engdegård
  2 siblings, 0 replies; 21+ messages in thread
From: Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-04-28 10:27 UTC (permalink / raw)
  To: Yuri D'Elia; +Cc: 63142


Yuri D'Elia <wavexx@thregr.org> writes:

> If the environment variable QUOTING_STYLE is set, GNU "ls" can return a
> format that dired doesn't like.
>
> QUOTING_STYLE=shell will quote files with spaces such as 'a b', and
> trying to interact with these files with dired results in:
>
> dired-get-file-for-visit: File no longer exists; type g to update Dired buffer
>
> Interestingly, even using "ls --dired" does the same if QUOTING_STYLE is
> set.

Are you suggesting that for ls, passing --dired should also override the
quoting style?  Either way, we can also pass --quoting-style onto the
command line to override it to something dired can play with.

> Should dired handle QUOTING_STYLE (looking at the coreutils manual it
> seems that other tools besides ls handle it) or should dired actively
> suppress the quoting mode?

-- 
Best,


RY

[Please note that this mail might go to spam due to some
misconfiguration in my mail server -- still investigating.]





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-28  9:35 bug#63142: 30.0.50; QUOTING_STYLE can break dired Yuri D'Elia
  2023-04-28 10:27 ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-04-28 10:28 ` Eli Zaretskii
  2023-04-28 10:37   ` Yuri D'Elia
  2023-05-05 11:04 ` Mattias Engdegård
  2 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2023-04-28 10:28 UTC (permalink / raw)
  To: Yuri D'Elia; +Cc: 63142

> From: Yuri D'Elia <wavexx@thregr.org>
> Date: Fri, 28 Apr 2023 11:35:09 +0200
> 
> 
> If the environment variable QUOTING_STYLE is set, GNU "ls" can return a
> format that dired doesn't like.
> 
> QUOTING_STYLE=shell will quote files with spaces such as 'a b', and
> trying to interact with these files with dired results in:
> 
> dired-get-file-for-visit: File no longer exists; type g to update Dired buffer
> 
> Interestingly, even using "ls --dired" does the same if QUOTING_STYLE is
> set.
> 
> Should dired handle QUOTING_STYLE (looking at the coreutils manual it
> seems that other tools besides ls handle it) or should dired actively
> suppress the quoting mode?

I'd suggest first to report this to Coreutils maintainers: it makes
little sense to me to honor QUOTING_STYLE when --dired is specified.
If the Coreutils folks decline to handle this, then I guess we will
have to.

Thanks.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-28 10:28 ` Eli Zaretskii
@ 2023-04-28 10:37   ` Yuri D'Elia
  2023-04-28 10:46     ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Yuri D'Elia @ 2023-04-28 10:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 63142

On Fri, Apr 28 2023, Eli Zaretskii wrote:
>> Should dired handle QUOTING_STYLE (looking at the coreutils manual it
>> seems that other tools besides ls handle it) or should dired actively
>> suppress the quoting mode?
>
> I'd suggest first to report this to Coreutils maintainers: it makes
> little sense to me to honor QUOTING_STYLE when --dired is specified.
> If the Coreutils folks decline to handle this, then I guess we will
> have to.

What sort of output should ls use in --dired mode?

literal?

From the documentation it seems that '-b' should be used to interact
with newlines, so some form of quoting might still be wanted?





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-28 10:37   ` Yuri D'Elia
@ 2023-04-28 10:46     ` Eli Zaretskii
  2023-04-28 18:01       ` Yuri D'Elia
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2023-04-28 10:46 UTC (permalink / raw)
  To: Yuri D'Elia; +Cc: 63142

> From: Yuri D'Elia <wavexx@thregr.org>
> Cc: 63142@debbugs.gnu.org
> Date: Fri, 28 Apr 2023 12:37:38 +0200
> 
> On Fri, Apr 28 2023, Eli Zaretskii wrote:
> >> Should dired handle QUOTING_STYLE (looking at the coreutils manual it
> >> seems that other tools besides ls handle it) or should dired actively
> >> suppress the quoting mode?
> >
> > I'd suggest first to report this to Coreutils maintainers: it makes
> > little sense to me to honor QUOTING_STYLE when --dired is specified.
> > If the Coreutils folks decline to handle this, then I guess we will
> > have to.
> 
> What sort of output should ls use in --dired mode?

The default one, I think.  --dired already produces the information
which tells where each file name begins and ends, so any quoting
sounds redundant, no?





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-28 10:46     ` Eli Zaretskii
@ 2023-04-28 18:01       ` Yuri D'Elia
  2023-04-29 10:46         ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Yuri D'Elia @ 2023-04-28 18:01 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 63142

On Fri, Apr 28 2023, Eli Zaretskii wrote:
>> > I'd suggest first to report this to Coreutils maintainers: it makes
>> > little sense to me to honor QUOTING_STYLE when --dired is specified.
>> > If the Coreutils folks decline to handle this, then I guess we will
>> > have to.
>>
>> What sort of output should ls use in --dired mode?
>
> The default one, I think.  --dired already produces the information
> which tells where each file name begins and ends, so any quoting
> sounds redundant, no?

So searching through coreutils I find this old report:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=23024

| On further analysis I see that --dired mode already distinguishes
| the quoting-style in its output.  Also we had already documented
| that --dired should specify a --quoting-style to get consistent output.

Which huh, does mention this in the ls manual:

| If you use a quoting style like ‘--quoting-style=c’ (‘-Q’) that
|  adds quote marks, then the offsets include the quote marks.  So
|  beware that the user may select the quoting style via the
|  environment variable ‘QUOTING_STYLE’.  Hence, applications using
|  ‘--dired’ should either specify an explicit
|  ‘--quoting-style=literal’ (‘-N’) option on the command line, or
|  else be prepared to parse the escaped names.

Then it references a commit mentioning an emacs fix:

https://github.com/emacs-mirror/emacs/commit/e4adb6

which however seems to apply to tramp only.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-28 18:01       ` Yuri D'Elia
@ 2023-04-29 10:46         ` Eli Zaretskii
  2023-04-29 15:05           ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2023-04-29 10:46 UTC (permalink / raw)
  To: Yuri D'Elia; +Cc: 63142

> From: Yuri D'Elia <wavexx@thregr.org>
> Cc: 63142@debbugs.gnu.org
> Date: Fri, 28 Apr 2023 20:01:01 +0200
> 
> So searching through coreutils I find this old report:
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=23024
> 
> | On further analysis I see that --dired mode already distinguishes
> | the quoting-style in its output.  Also we had already documented
> | that --dired should specify a --quoting-style to get consistent output.
> 
> Which huh, does mention this in the ls manual:
> 
> | If you use a quoting style like ‘--quoting-style=c’ (‘-Q’) that
> |  adds quote marks, then the offsets include the quote marks.  So
> |  beware that the user may select the quoting style via the
> |  environment variable ‘QUOTING_STYLE’.  Hence, applications using
> |  ‘--dired’ should either specify an explicit
> |  ‘--quoting-style=literal’ (‘-N’) option on the command line, or
> |  else be prepared to parse the escaped names.
> 
> Then it references a commit mentioning an emacs fix:
> 
> https://github.com/emacs-mirror/emacs/commit/e4adb6
> 
> which however seems to apply to tramp only.

Does the patch below give good results?

diff --git a/lisp/dired.el b/lisp/dired.el
index d1471e9..e3a9d7b 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -1653,7 +1653,10 @@ dired-insert-directory
 see `dired-use-ls-dired' for more details.")
 		       nil))
 	       dired-use-ls-dired)))
-	(setq switches (concat "--dired " switches)))
+        ;; Use -N with --dired, to countermand possible non-default
+        ;; quoting style, in particular via the environment variable
+        ;; QUOTINTG_STYLE.
+	(setq switches (concat "--dired -N " switches)))
     ;; Expand directory wildcards and fill file-list.
     (let ((dir-wildcard (insert-directory-wildcard-in-dir-p dir)))
       (cond (dir-wildcard





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-29 10:46         ` Eli Zaretskii
@ 2023-04-29 15:05           ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-04-29 15:16             ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-04-29 15:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Yuri D'Elia, 63142


Eli Zaretskii <eliz@gnu.org> writes:

> Does the patch below give good results?
>
> diff --git a/lisp/dired.el b/lisp/dired.el
> index d1471e9..e3a9d7b 100644
> --- a/lisp/dired.el
> +++ b/lisp/dired.el
> @@ -1653,7 +1653,10 @@ dired-insert-directory
>  see `dired-use-ls-dired' for more details.")
>  		       nil))
>  	       dired-use-ls-dired)))
> -	(setq switches (concat "--dired " switches)))
> +        ;; Use -N with --dired, to countermand possible non-default
> +        ;; quoting style, in particular via the environment variable
> +        ;; QUOTINTG_STYLE.
> +	(setq switches (concat "--dired -N " switches)))
>      ;; Expand directory wildcards and fill file-list.
>      (let ((dir-wildcard (insert-directory-wildcard-in-dir-p dir)))
>        (cond (dir-wildcard

Do we need to prevent "wild" users from setting a conflicting quoting
style in `switches', in which case we should swap the two arguments to
`concat'?

-- 
Best,


RY

[Please note that this mail might go to spam due to some
misconfiguration in my mail server -- still investigating.]





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-29 15:05           ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-04-29 15:16             ` Eli Zaretskii
  2023-05-02  9:35               ` Yuri D'Elia
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2023-04-29 15:16 UTC (permalink / raw)
  To: Ruijie Yu; +Cc: wavexx, 63142

> From: Ruijie Yu <ruijie@netyu.xyz>
> Cc: Yuri D'Elia <wavexx@thregr.org>, 63142@debbugs.gnu.org
> Date: Sat, 29 Apr 2023 23:05:27 +0800
> 
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Does the patch below give good results?
> >
> > diff --git a/lisp/dired.el b/lisp/dired.el
> > index d1471e9..e3a9d7b 100644
> > --- a/lisp/dired.el
> > +++ b/lisp/dired.el
> > @@ -1653,7 +1653,10 @@ dired-insert-directory
> >  see `dired-use-ls-dired' for more details.")
> >  		       nil))
> >  	       dired-use-ls-dired)))
> > -	(setq switches (concat "--dired " switches)))
> > +        ;; Use -N with --dired, to countermand possible non-default
> > +        ;; quoting style, in particular via the environment variable
> > +        ;; QUOTINTG_STYLE.
> > +	(setq switches (concat "--dired -N " switches)))
> >      ;; Expand directory wildcards and fill file-list.
> >      (let ((dir-wildcard (insert-directory-wildcard-in-dir-p dir)))
> >        (cond (dir-wildcard
> 
> Do we need to prevent "wild" users from setting a conflicting quoting
> style in `switches'

Why would we want to do that?  If they think they know what they are
doing, let them live with the consequences.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-29 15:16             ` Eli Zaretskii
@ 2023-05-02  9:35               ` Yuri D'Elia
  2023-05-02 12:18                 ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Yuri D'Elia @ 2023-05-02  9:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Ruijie Yu, 63142

On Sat, Apr 29 2023, Eli Zaretskii wrote:
>> > +	(setq switches (concat "--dired -N " switches)))
>> >      ;; Expand directory wildcards and fill file-list.
>> >      (let ((dir-wildcard (insert-directory-wildcard-in-dir-p dir)))
>> >        (cond (dir-wildcard
>>
>> Do we need to prevent "wild" users from setting a conflicting quoting
>> style in `switches'
>
> Why would we want to do that?  If they think they know what they are
> doing, let them live with the consequences.

It's always a good idea to actually have the possibility to override
flags as an escape hatch.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-02  9:35               ` Yuri D'Elia
@ 2023-05-02 12:18                 ` Eli Zaretskii
  2023-05-02 15:57                   ` Yuri D'Elia
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2023-05-02 12:18 UTC (permalink / raw)
  To: Yuri D'Elia; +Cc: ruijie, 63142

> From: Yuri D'Elia <wavexx@thregr.org>
> Cc: Ruijie Yu <ruijie@netyu.xyz>, 63142@debbugs.gnu.org
> Date: Tue, 02 May 2023 11:35:13 +0200
> 
> On Sat, Apr 29 2023, Eli Zaretskii wrote:
> >> > +	(setq switches (concat "--dired -N " switches)))
> >> >      ;; Expand directory wildcards and fill file-list.
> >> >      (let ((dir-wildcard (insert-directory-wildcard-in-dir-p dir)))
> >> >        (cond (dir-wildcard
> >>
> >> Do we need to prevent "wild" users from setting a conflicting quoting
> >> style in `switches'
> >
> > Why would we want to do that?  If they think they know what they are
> > doing, let them live with the consequences.
> 
> It's always a good idea to actually have the possibility to override
> flags as an escape hatch.

I agree.

What about the patch I proposed: does it give good results?





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-02 12:18                 ` Eli Zaretskii
@ 2023-05-02 15:57                   ` Yuri D'Elia
  2023-05-02 17:48                     ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Yuri D'Elia @ 2023-05-02 15:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ruijie, 63142

On Tue, May 02 2023, Eli Zaretskii wrote:
> What about the patch I proposed: does it give good results?

Working as intended.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-02 15:57                   ` Yuri D'Elia
@ 2023-05-02 17:48                     ` Eli Zaretskii
  0 siblings, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2023-05-02 17:48 UTC (permalink / raw)
  To: Yuri D'Elia; +Cc: ruijie, 63142-done

> From: Yuri D'Elia <wavexx@thregr.org>
> Cc: ruijie@netyu.xyz, 63142@debbugs.gnu.org
> Date: Tue, 02 May 2023 17:57:50 +0200
> 
> On Tue, May 02 2023, Eli Zaretskii wrote:
> > What about the patch I proposed: does it give good results?
> 
> Working as intended.

Thanks, so I've now installed it on the master branch, and I'm closing
this bug.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-04-28  9:35 bug#63142: 30.0.50; QUOTING_STYLE can break dired Yuri D'Elia
  2023-04-28 10:27 ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-04-28 10:28 ` Eli Zaretskii
@ 2023-05-05 11:04 ` Mattias Engdegård
  2023-05-05 11:06   ` Yuri D'Elia
  2023-05-05 11:25   ` Eli Zaretskii
  2 siblings, 2 replies; 21+ messages in thread
From: Mattias Engdegård @ 2023-05-05 11:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Yuri D'Elia, ruijie, 63142, Michael Albinus

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

This change breaks tramp-tests on MacOS (and, I presume, BSDs) where `ls` accepts neither `--dired` nor `-N`. see appended test log.

In fact even ls-lisp doesn't seem to accept `-N`, or at least do anything with it. Not sure if that is a problem or not.

Should we reopen this bug or open a new one?


[-- Attachment #2: tramp-tests.log --]
[-- Type: application/octet-stream, Size: 6366 bytes --]

Running 57 tests (2023-05-04 17:59:47+0200, selector `(not (or (tag :expensive-test) (tag :unstable) (tag :nativecomp)))')
Remote directory: `/mock::/var/folders/qy/zstv16390nlcs47kz8nff_mm0000gn/T/'
   passed   1/57  tramp-test00-availability (0.196691 sec)
   passed   2/57  tramp-test01-file-name-syntax (0.024810 sec)
   passed   3/57  tramp-test02-file-name-dissect (0.012798 sec)
   passed   4/57  tramp-test03-file-name-defaults (0.013957 sec)
   passed   5/57  tramp-test03-file-name-host-rules (0.135664 sec)
   passed   6/57  tramp-test03-file-name-method-rules (0.267525 sec)
   passed   7/57  tramp-test04-substitute-in-file-name (0.016100 sec)
   passed   8/57  tramp-test05-expand-file-name (0.000603 sec)
   passed   9/57  tramp-test05-expand-file-name-relative (0.250355 sec)
   passed  10/57  tramp-test05-expand-file-name-top (0.246213 sec)
   passed  11/57  tramp-test06-directory-file-name (0.211605 sec)
   passed  12/57  tramp-test07-abbreviate-file-name (0.863962 sec)
   passed  13/57  tramp-test07-file-exists-p (0.623836 sec)
   passed  14/57  tramp-test08-file-local-copy (0.796964 sec)
   passed  15/57  tramp-test09-insert-file-contents (1.054375 sec)
Wrote /mock:stanniol.local:/var/folders/qy/zstv16390nlcs47kz8nff_mm0000gn/T/tramp-testTgbUDR
Wrote /mock:stanniol.local:/var/folders/qy/zstv16390nlcs47kz8nff_mm0000gn/T/tramp-testTgbUDR
Wrote /mock:stanniol.local:/var/folders/qy/zstv16390nlcs47kz8nff_mm0000gn/T/tramp-testTgbUDR
   passed  16/57  tramp-test10-write-region (1.979867 sec)
   passed  17/57  tramp-test10-write-region-file-precious-flag (1.285421 sec)
  skipped  18/57  tramp-test10-write-region-other-file-name-handler (0.023221 sec)
   passed  19/57  tramp-test11-copy-file (3.803086 sec)
   passed  20/57  tramp-test12-rename-file (4.470811 sec)
   passed  21/57  tramp-test13-make-directory (0.435704 sec)
   passed  22/57  tramp-test14-delete-directory (0.686477 sec)
   passed  23/57  tramp-test15-copy-directory (2.086941 sec)
   passed  24/57  tramp-test16-directory-files (1.464579 sec)
   passed  25/57  tramp-test16-file-expand-wildcards (0.728717 sec)
Test tramp-test17-dired-with-wildcards backtrace:
  re-search-forward("tramp-testxyIR7k")
  apply(re-search-forward "tramp-testxyIR7k")
  #f(compiled-function () #<bytecode -0x167d9f2a3e13d219>)()
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name tramp-test17-dired-with-wildcards :do
  ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
  ert-run-tests((not (or (tag :expensive-test) (tag :unstable) (tag :n
  ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable) (
  ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
  eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
  command-line-1(("-L" ":../../emacs/test" "-l" "ert" "-l" "lisp/net/t
  command-line()
  normal-top-level()
Test tramp-test17-dired-with-wildcards condition:
    (search-failed "tramp-testxyIR7k")
   FAILED  26/57  tramp-test17-dired-with-wildcards (0.618446 sec) at ../../emacs/test/lisp/net/tramp-tests.el:3316
   passed  27/57  tramp-test17-insert-directory (0.626790 sec)
Test tramp-test17-insert-directory-one-file backtrace:
  signal(ert-test-failed (((should-not (eobp)) :form (eobp) :value t))
  ert-fail(((should-not (eobp)) :form (eobp) :value t))
  #f(compiled-function () #<bytecode -0x22e0df6cebf0d13>)()
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name tramp-test17-insert-directory-one-fil
  ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
  ert-run-tests((not (or (tag :expensive-test) (tag :unstable) (tag :n
  ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable) (
  ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
  eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
  command-line-1(("-L" ":../../emacs/test" "-l" "ert" "-l" "lisp/net/t
  command-line()
  normal-top-level()
Test tramp-test17-insert-directory-one-file condition:
    (ert-test-failed
     ((should-not
       (eobp))
      :form
      (eobp)
      :value t))
   FAILED  28/57  tramp-test17-insert-directory-one-file (0.518754 sec) at ../../emacs/test/lisp/net/tramp-tests.el:3431
   passed  29/57  tramp-test18-file-attributes (1.514951 sec)
   passed  30/57  tramp-test19-directory-files-and-attributes (0.780391 sec)
   passed  31/57  tramp-test20-file-modes (0.496595 sec)
   passed  32/57  tramp-test21-file-links (1.986896 sec)
   passed  33/57  tramp-test22-file-times (0.677137 sec)
   passed  34/57  tramp-test23-visited-file-modtime (0.486873 sec)
  skipped  35/57  tramp-test24-file-acl (0.170199 sec)
  skipped  36/57  tramp-test25-file-selinux (0.211741 sec)
   passed  37/57  tramp-test26-file-name-completion (1.384207 sec)
   passed  38/57  tramp-test26-interactive-file-name-completion (2.009675 sec)
   passed  39/57  tramp-test27-load (0.838852 sec)
   passed  40/57  tramp-test33-environment-variables-and-port-numbers (0.771784 sec)
   passed  41/57  tramp-test35-exec-path (1.303049 sec)
   passed  42/57  tramp-test35-remote-path (3.657313 sec)
   passed  43/57  tramp-test37-make-auto-save-file-name (0.594019 sec)
   passed  44/57  tramp-test38-find-backup-file-name (0.781815 sec)
   passed  45/57  tramp-test39-detect-external-change (3.815419 sec)
   passed  46/57  tramp-test39-make-lock-file-name (4.178823 sec)
   passed  47/57  tramp-test40-make-nearby-temp-file (1.329095 sec)
   passed  48/57  tramp-test41-special-characters (2.279381 sec)
   passed  49/57  tramp-test42-utf8 (6.554795 sec)
   passed  50/57  tramp-test43-file-system-info (0.164572 sec)
   passed  51/57  tramp-test44-file-user-group-ids (0.276660 sec)
  skipped  52/57  tramp-test46-dired-compress-dir (0.099935 sec)
  skipped  53/57  tramp-test46-dired-compress-file (0.219227 sec)
   passed  54/57  tramp-test48-auto-load (0.544248 sec)
   passed  55/57  tramp-test48-delay-load (0.299999 sec)
   passed  56/57  tramp-test48-recursive-load (0.676982 sec)
   passed  57/57  tramp-test48-remote-load-path (0.252166 sec)

Ran 57 tests, 50 results as expected, 2 unexpected, 5 skipped (2023-05-04 18:00:49+0200, 62.139392 sec)

2 unexpected results:
   FAILED  tramp-test17-dired-with-wildcards
   FAILED  tramp-test17-insert-directory-one-file


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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-05 11:04 ` Mattias Engdegård
@ 2023-05-05 11:06   ` Yuri D'Elia
  2023-05-05 11:25   ` Eli Zaretskii
  1 sibling, 0 replies; 21+ messages in thread
From: Yuri D'Elia @ 2023-05-05 11:06 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: ruijie, Eli Zaretskii, 63142, Michael Albinus

On Fri, May 05 2023, Mattias Engdegård wrote:
> This change breaks tramp-tests on MacOS (and, I presume, BSDs) where
> `ls` accepts neither `--dired` nor `-N`. see appended test log.

Considering -N was added where --dired was already being used, I'm kind
of surprised?





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-05 11:04 ` Mattias Engdegård
  2023-05-05 11:06   ` Yuri D'Elia
@ 2023-05-05 11:25   ` Eli Zaretskii
  2023-05-05 12:03     ` Michael Albinus
  1 sibling, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2023-05-05 11:25 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: wavexx, ruijie, 63142, michael.albinus

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Fri, 5 May 2023 13:04:56 +0200
> Cc: Michael Albinus <michael.albinus@gmx.de>,
>  Yuri D'Elia <wavexx@thregr.org>,
>  ruijie@netyu.xyz,
>  63142@debbugs.gnu.org
> 
> This change breaks tramp-tests on MacOS (and, I presume, BSDs) where `ls` accepts neither `--dired` nor `-N`. see appended test log.

Sorry, I don't understand.  The change added -N only where we
previously had --dired in the 'ls' switches.  Are you saying that we
somehow now pass "--dired -N" where previously we didn't pass "--dired"?

AFAIU, we should use --dired only when 'ls' supports it, and we now
use -N under the same conditions.  Is that not what happens?

> In fact even ls-lisp doesn't seem to accept `-N`, or at least do anything with it. Not sure if that is a problem or not.

This should not be used when ls-lisp is in use, see the line
emphasized below:

    (if (and
	 ;; Don't try to invoke `ls' if we are on DOS/Windows where
	 ;; ls-lisp emulation is used, except if they want to use `ls'
	 ;; as indicated by `ls-lisp-use-insert-directory-program'.
	 (not (and (featurep 'ls-lisp)  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
		   (null ls-lisp-use-insert-directory-program)))
         ;; FIXME: Big ugly hack for Eshell's eshell-ls-use-in-dired.
         (not (bound-and-true-p eshell-ls-use-in-dired))
	 (or (file-remote-p dir)
             (if (eq dired-use-ls-dired 'unspecified)
		 ;; Check whether "ls --dired" gives exit code 0, and
		 ;; save the answer in `dired-use-ls-dired'.
		 (or (setq dired-use-ls-dired
			   (eq 0 (call-process insert-directory-program
                                               nil nil nil "--dired")))
		     (progn
		       (message "ls does not support --dired; \
see `dired-use-ls-dired' for more details.")
		       nil))
	       dired-use-ls-dired)))
        ;; Use -N with --dired, to countermand possible non-default
        ;; quoting style, in particular via the environment variable
        ;; QUOTING_STYLE.
	(setq switches (concat "--dired -N " switches)))

So once again, I don't understand what you are saying.  In any case,
I've just tried "C-x d" on MS-Windows, where ls-lisp is used by
default, and saw no problems.  What am I missing?

> Should we reopen this bug or open a new one?

I'd like Michael to chime in first.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-05 11:25   ` Eli Zaretskii
@ 2023-05-05 12:03     ` Michael Albinus
  2023-05-05 12:11       ` Mattias Engdegård
  2023-05-05 13:28       ` Eli Zaretskii
  0 siblings, 2 replies; 21+ messages in thread
From: Michael Albinus @ 2023-05-05 12:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: wavexx, ruijie, Mattias Engdegård, 63142

Eli Zaretskii <eliz@gnu.org> writes:

Hi,

>> This change breaks tramp-tests on MacOS (and, I presume, BSDs) where `ls` accepts neither `--dired` nor `-N`. see appended test log.
>
> AFAIU, we should use --dired only when 'ls' supports it, and we now
> use -N under the same conditions.  Is that not what happens?

This still happens.

>> Should we reopen this bug or open a new one?
>
> I'd like Michael to chime in first.

Well, we're speaking about tramp-sh-handle-insert-directory. This does
the following:

- Check, whether the remote ls command supports "--quoting-style=literal
  --show-control-chars" or "-w" (for *BSD). If yes, add this to SWITCHES.

- Check, whether the remote ls command supports "--dired". If not, remove
  "--dired" from SWITCHES.

It doesn't know anything about "-N". So I propose to remove "-N" from
switches unconditionally in Tramp.

Best regards, Michael.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-05 12:03     ` Michael Albinus
@ 2023-05-05 12:11       ` Mattias Engdegård
  2023-05-05 14:44         ` Michael Albinus
  2023-05-05 13:28       ` Eli Zaretskii
  1 sibling, 1 reply; 21+ messages in thread
From: Mattias Engdegård @ 2023-05-05 12:11 UTC (permalink / raw)
  To: Michael Albinus; +Cc: wavexx, ruijie, Eli Zaretskii, 63142

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

5 maj 2023 kl. 14.03 skrev Michael Albinus <michael.albinus@gmx.de>:

> It doesn't know anything about "-N". So I propose to remove "-N" from
> switches unconditionally in Tramp.

Yes, I found that out the hard way and can confirm that the patch below makes tramp-tests pass again.
Whether it's the right course of action, or requires more work elsewhere, I leave to you and Eli.


[-- Attachment #2: bsd-ls-N.diff --]
[-- Type: application/octet-stream, Size: 1774 bytes --]

diff --git a/lisp/dired.el b/lisp/dired.el
index 1c8d011d765..e70467ca53b 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -1647,9 +1647,9 @@ dired-insert-directory
 		 ;; save the answer in `dired-use-ls-dired'.
 		 (or (setq dired-use-ls-dired
 			   (eq 0 (call-process insert-directory-program
-                                               nil nil nil "--dired")))
+                                               nil nil nil "--dired" "-N")))
 		     (progn
-		       (message "ls does not support --dired; \
+		       (message "ls does not support --dired -N; \
 see `dired-use-ls-dired' for more details.")
 		       nil))
 	       dired-use-ls-dired)))
@@ -1665,7 +1665,7 @@ dired-insert-directory
              ;; "--dired", so we cannot add it to the `process-file'
              ;; call for wildcards.
              (when (file-remote-p dir)
-               (setq switches (string-replace "--dired" "" switches)))
+               (setq switches (string-replace "--dired -N" "" switches)))
              (let* ((default-directory (car dir-wildcard))
                     (script (format "ls %s %s" switches (cdr dir-wildcard)))
                     (remotep (file-remote-p dir))
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 94fbc588b5d..d020615af07 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2566,7 +2566,7 @@ tramp-sh-handle-insert-directory
       (setq switches
 	    (append switches (split-string (tramp-sh--quoting-style-options v))))
       (unless (tramp-get-ls-command-with v "--dired")
-	(setq switches (delete "--dired" switches)))
+	(setq switches (delete "-N" (delete "--dired" switches))))
       (when wildcard
         (setq wildcard (tramp-run-real-handler
 			#'file-name-nondirectory (list localname)))

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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-05 12:03     ` Michael Albinus
  2023-05-05 12:11       ` Mattias Engdegård
@ 2023-05-05 13:28       ` Eli Zaretskii
  1 sibling, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2023-05-05 13:28 UTC (permalink / raw)
  To: Michael Albinus; +Cc: wavexx, ruijie, mattias.engdegard, 63142

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Mattias Engdegård <mattias.engdegard@gmail.com>,
>   wavexx@thregr.org,
>   ruijie@netyu.xyz,  63142@debbugs.gnu.org
> Date: Fri, 05 May 2023 14:03:13 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Should we reopen this bug or open a new one?
> >
> > I'd like Michael to chime in first.
> 
> Well, we're speaking about tramp-sh-handle-insert-directory. This does
> the following:
> 
> - Check, whether the remote ls command supports "--quoting-style=literal
>   --show-control-chars" or "-w" (for *BSD). If yes, add this to SWITCHES.
> 
> - Check, whether the remote ls command supports "--dired". If not, remove
>   "--dired" from SWITCHES.
> 
> It doesn't know anything about "-N". So I propose to remove "-N" from
> switches unconditionally in Tramp.

SGTM, thanks.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-05 12:11       ` Mattias Engdegård
@ 2023-05-05 14:44         ` Michael Albinus
  2023-05-05 17:26           ` Mattias Engdegård
  0 siblings, 1 reply; 21+ messages in thread
From: Michael Albinus @ 2023-05-05 14:44 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: wavexx, ruijie, Eli Zaretskii, 63142

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

Hi Mattias,

> Yes, I found that out the hard way and can confirm that the patch below makes tramp-tests pass again.
> Whether it's the right course of action, or requires more work elsewhere, I leave to you and Eli.

Thanks. LGTM, so you might push it.

It doesn't handle the case that somebody uses switches like "-alN". But
this would be intentional by the user, and we don't need to care about.

Best regards, Michael.





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

* bug#63142: 30.0.50; QUOTING_STYLE can break dired
  2023-05-05 14:44         ` Michael Albinus
@ 2023-05-05 17:26           ` Mattias Engdegård
  0 siblings, 0 replies; 21+ messages in thread
From: Mattias Engdegård @ 2023-05-05 17:26 UTC (permalink / raw)
  To: Michael Albinus; +Cc: wavexx, ruijie, Eli Zaretskii, 63142

5 maj 2023 kl. 16.44 skrev Michael Albinus <michael.albinus@gmx.de>:

> Thanks. LGTM, so you might push it.

Thank you, now on master.






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

end of thread, other threads:[~2023-05-05 17:26 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-28  9:35 bug#63142: 30.0.50; QUOTING_STYLE can break dired Yuri D'Elia
2023-04-28 10:27 ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-04-28 10:28 ` Eli Zaretskii
2023-04-28 10:37   ` Yuri D'Elia
2023-04-28 10:46     ` Eli Zaretskii
2023-04-28 18:01       ` Yuri D'Elia
2023-04-29 10:46         ` Eli Zaretskii
2023-04-29 15:05           ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-04-29 15:16             ` Eli Zaretskii
2023-05-02  9:35               ` Yuri D'Elia
2023-05-02 12:18                 ` Eli Zaretskii
2023-05-02 15:57                   ` Yuri D'Elia
2023-05-02 17:48                     ` Eli Zaretskii
2023-05-05 11:04 ` Mattias Engdegård
2023-05-05 11:06   ` Yuri D'Elia
2023-05-05 11:25   ` Eli Zaretskii
2023-05-05 12:03     ` Michael Albinus
2023-05-05 12:11       ` Mattias Engdegård
2023-05-05 14:44         ` Michael Albinus
2023-05-05 17:26           ` Mattias Engdegård
2023-05-05 13:28       ` Eli Zaretskii

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.