GNU bug report logs - #70271
30.0.50; [PATCH] * lisp/ls-lisp.el (ls-lisp--sanitize-switches): support more

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 70271 in the body.
You can then email your comments to 70271 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7.mail <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; [PATCH] * lisp/ls-lisp.el (ls-lisp--sanitize-switches):
 support more
Date: Mon, 8 Apr 2024 05:10:22 +0000

[Message part 1 (text/plain, inline)]

Hi,

The dired ignored the options like "--time=ctime" in the variable
`dired-listing-switches' on Windows OS.

 I traced the code and found dired will use the functions in the
`ls-lisp.el' instead of calling the real command "/usr/bin/ls", and
the "ls-lisp.el" didn't process the "--time=ctime" option.

The attached PATCH will convert the "--time=ctime" to "-c"... in the
"ls-lisp.el", and then the dired will display the desired time in its
time column.

Here is the verification code (on Windows OS), try it one line and
close the dired buffer then try next line.

(dired "/tmp/aa" "-al --sort=time") ; as "-al -t"

(dired "/tmp/aa" "-al --time=ctime") ; as "al -c"

(dired "/tmp/aa" "-al --time=atime") ; as "al -u"

Please help review the changes and approve them. Thanks

[0001-lisp-ls-lisp.el-ls-lisp-sanitize-switches-support-mo.patch (text/x-patch, attachment)]

Message #8 received at 70271 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lin Sun <sunlin7.mail <at> gmail.com>
Cc: 70271 <at> debbugs.gnu.org
Subject: Re: bug#70271: 30.0.50;
 [PATCH] * lisp/ls-lisp.el (ls-lisp--sanitize-switches): support more
Date: Mon, 08 Apr 2024 14:25:02 +0300

> From: Lin Sun <sunlin7.mail <at> gmail.com>
> Date: Mon, 8 Apr 2024 05:10:22 +0000
> 
> The dired ignored the options like "--time=ctime" in the variable
> `dired-listing-switches' on Windows OS.
> 
>  I traced the code and found dired will use the functions in the
> `ls-lisp.el' instead of calling the real command "/usr/bin/ls", and
> the "ls-lisp.el" didn't process the "--time=ctime" option.
> 
> The attached PATCH will convert the "--time=ctime" to "-c"... in the
> "ls-lisp.el", and then the dired will display the desired time in its
> time column.
> 
> Here is the verification code (on Windows OS), try it one line and
> close the dired buffer then try next line.
> 
> (dired "/tmp/aa" "-al --sort=time") ; as "-al -t"
> 
> (dired "/tmp/aa" "-al --time=ctime") ; as "al -c"
> 
> (dired "/tmp/aa" "-al --time=atime") ; as "al -u"
> 
> Please help review the changes and approve them. Thanks

Thanks, but how about adding some tests for this?

Message #11 received at 70271 <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7.mail <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70271 <at> debbugs.gnu.org
Subject: Re: bug#70271: 30.0.50;
 [PATCH] * lisp/ls-lisp.el (ls-lisp--sanitize-switches): support more
Date: Mon, 8 Apr 2024 20:43:54 +0000

[Message part 1 (text/plain, inline)]

On Mon, Apr 8, 2024 at 11:25 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Lin Sun <sunlin7.mail <at> gmail.com>
> > Date: Mon, 8 Apr 2024 05:10:22 +0000
> >
> > The dired ignored the options like "--time=ctime" in the variable
> > `dired-listing-switches' on Windows OS.
> >
> >  I traced the code and found dired will use the functions in the
> > `ls-lisp.el' instead of calling the real command "/usr/bin/ls", and
> > the "ls-lisp.el" didn't process the "--time=ctime" option.
> >
> > The attached PATCH will convert the "--time=ctime" to "-c"... in the
> > "ls-lisp.el", and then the dired will display the desired time in its
> > time column.
> >
> > Here is the verification code (on Windows OS), try it one line and
> > close the dired buffer then try next line.
> >
> > (dired "/tmp/aa" "-al --sort=time") ; as "-al -t"
> >
> > (dired "/tmp/aa" "-al --time=ctime") ; as "al -c"
> >
> > (dired "/tmp/aa" "-al --time=atime") ; as "al -u"
> >
> > Please help review the changes and approve them. Thanks
>
> Thanks, but how about adding some tests for this?

Sure, I had added the test case for this ticket.

Changes are attached in the PATCH.

Please help review again. Thanks.

[0001-Support-more-time-options-for-ls-lisp-sanitize-switc.patch (text/x-patch, attachment)]

Message #16 received at 70271-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lin Sun <sunlin7.mail <at> gmail.com>
Cc: 70271-done <at> debbugs.gnu.org
Subject: Re: bug#70271: 30.0.50;
 [PATCH] * lisp/ls-lisp.el (ls-lisp--sanitize-switches): support more
Date: Tue, 09 Apr 2024 06:24:23 +0300

> From: Lin Sun <sunlin7.mail <at> gmail.com>
> Date: Mon, 8 Apr 2024 20:43:54 +0000
> Cc: 70271 <at> debbugs.gnu.org
> 
> On Mon, Apr 8, 2024 at 11:25 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> > > From: Lin Sun <sunlin7.mail <at> gmail.com>
> > > Date: Mon, 8 Apr 2024 05:10:22 +0000
> > >
> > > The dired ignored the options like "--time=ctime" in the variable
> > > `dired-listing-switches' on Windows OS.
> > >
> > >  I traced the code and found dired will use the functions in the
> > > `ls-lisp.el' instead of calling the real command "/usr/bin/ls", and
> > > the "ls-lisp.el" didn't process the "--time=ctime" option.
> > >
> > > The attached PATCH will convert the "--time=ctime" to "-c"... in the
> > > "ls-lisp.el", and then the dired will display the desired time in its
> > > time column.
> > >
> > > Here is the verification code (on Windows OS), try it one line and
> > > close the dired buffer then try next line.
> > >
> > > (dired "/tmp/aa" "-al --sort=time") ; as "-al -t"
> > >
> > > (dired "/tmp/aa" "-al --time=ctime") ; as "al -c"
> > >
> > > (dired "/tmp/aa" "-al --time=atime") ; as "al -u"
> > >
> > > Please help review the changes and approve them. Thanks
> >
> > Thanks, but how about adding some tests for this?
> 
> Sure, I had added the test case for this ticket.
> 
> Changes are attached in the PATCH.

Thanks, installed on master, and closing the bug.

The test had a small bug, I fixed it.

Also, please look at how I modified the commit log message, and in the
future try following our conventions more closely (capitalization of
log entries, specifying function names, etc.)

Message #19 received at 70271-done <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7.mail <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70271-done <at> debbugs.gnu.org
Subject: Re: bug#70271: 30.0.50;
 [PATCH] * lisp/ls-lisp.el (ls-lisp--sanitize-switches): support more
Date: Tue, 9 Apr 2024 04:10:47 +0000

On Tue, Apr 9, 2024 at 3:24 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> Thanks, installed on master, and closing the bug.
> The test had a small bug, I fixed it.
> Also, please look at how I modified the commit log message, and in the
> future try following our conventions more closely (capitalization of
> log entries, specifying function names, etc.)

Thank you so much, appreciate it! And will follow code/log rules in
future patches.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.