Re: [Elpa] Suggest new package: dired-quick-sort

[Philip Kaludercic]

Hong Xu <hong@topbug.net> writes:

> On 2025-01-29 Wed 11:56 GMT-08, Philip Kaludercic <philipk@posteo.net> wrote:
>
>>
>> Sorry for the delay in responding; I have a few comments here:
>
> Thanks for the comment.
>
>> -;; Copyright (C) 2016--2025 Hong Xu <hong@topbug.net>
>> +;; Copyright (C) 2016-2025 Hong Xu <hong@topbug.net>
>
> Is this an Emacs convention? -- is more like n-dash while - is hyphen.

I don't recall ever seeing anyone use "--", certainly for Emacs
packages, perhaps even outside of it.

The way I see it, -- are two dashes.  TeX might interpret it as a ndash,
but this is just ASCII and there is no difference between hyphens,
n-dashes and m-dashes.  I don't know if anything would go wrong if you
would decide to write 

      ;; Copyright (C) 2016–2025 Hong Xu <hong@topbug.net>
                           ^
                           That's a "EN DASH".

>> +;;     (require 'dired-quick-sort) ;<- are you assuming that the autoloads 
>> are in effect or not?
>
> Yes, I'm assuming autoloads are in effect.

This this line in the suggested configuration shouldn't be necessary?

>>  (defun dired-quick-sort--format-switches ()
>> -  "Return a dired-listing-switches string according to
>> -`dired-quick-sort' settings."
>> -  (format "%s %s %s %s %s" dired-listing-switches
>> -          (if (string= dired-quick-sort-sort-by-last "default")
>> -              ""
>> -            (concat "--sort=" dired-quick-sort-sort-by-last))
>> -          (if (char-equal dired-quick-sort-reverse-last ?y)
>> -              "-r" "")
>> -          (if (char-equal dired-quick-sort-group-directories-last ?y)
>> -              "--group-directories-first" "")
>> -          (if (not (string= dired-quick-sort-time-last "default"))
>> -              (concat "--time=" dired-quick-sort-time-last) "")))
>> -
>> -(defun dired-quick-sort--sort-by-last (field)
>> +  "Return a `dired-listing-switches' string according to `dired-quick-sort' 
>> settings."
>> +  (mapconcat
>> +   #'identity                           ;perhaps `shell-quote-argument'?
>> +   (list dired-listing-switches
>> +         (if (string= dired-quick-sort-sort-by-last "default")
>> +             ""                         ;`mapconcat' interprets nil as "", 
>> which could simplify things here
>> +           (concat "--sort=" dired-quick-sort-sort-by-last))
>> +         (if (char-equal dired-quick-sort-reverse-last ?y) ;why not `eq'?
>> +             "-r" "")
>> +         (if (char-equal dired-quick-sort-group-directories-last ?y)
>> +             "--group-directories-first" "")
>> +         (if (not (string= dired-quick-sort-time-last "default"))
>> +             (concat "--time=" dired-quick-sort-time-last) ""))
>> +   " "))
>> +
>> +(defun dired-quick-sort--sort-by-last (field) ;please add a docstring!
>>    (if (string= dired-quick-sort-sort-by-last field) "[X]" "[ ]"))
>>  
>> +;; you don't appear to have a hard dependency on hydra, right?  could you 
>> also add a similar interface using transient?
>
> I think it's possible, but I don't have the time to revamp this part
> now. Is transient a hard requirement for adding to ELPA? If not, we can
> do it as an improvement in the future.

No certainly not.  If something was a hard requirement I would have
stated it explicitly, and not just left a comment.

>>  `dired-quick-sort-suppress-setup-warning' to suppress warning and skip setup
>>  silently.")
>>      (if (not
>> -         (with-temp-buffer
>> +         (with-temp-buffer              ;this could also be checked at 
>> compile-time, right?
>
> Yes, but I decided to check at runtime because when Emacs starts up, the
> environment may not be the same from last time. It can happen on a
> *BSD/MacOS system where people can install the GNU Coreutils after
> they've installed this package.

That's fair.

>>> I'm also open to include this package in the GNU Elpa repository if this
>>> is more preferred.
>>
>> That is fine, as long as all signficant contributors have signed the FSF
>> copyright assignment.
>
> How can I confirm if other signficant contributors have signed the
> copyright assignment? I believe there are two other significant
> contributors according to the git log, but I'm not sure whether one of
> them have signed.

Can you give me the names, and then we can have it checked?