GNU bug report logs - #35564
27.0.50; [PATCH] Tweak dired-do-shell-command warning about "wildcard" characters

Previous Next

Full log

View this message in rfc822 format

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 35564 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, npostavs <at> gmail.com
Subject: bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters
Date: Sun, 28 Jul 2019 01:32:19 +0200

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

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

>> -(defun dired--no-subst-prompt (char-positions command)
>> +(defun dired--mark-positions (positions)
>> +  (let ((markers (make-string
>> +                  (1+ (apply #'max positions))
>
> Is POSITIONS guaranteed to be non-nil?  (The max function takes at least
> one argument.)

AFAICT dired--mark-positions is only called by dired--no-subst-prompt,
which is only used when there is at least one ambiguous character to
highlight.

So as things stand now, POSITIONS will always be non-nil.  Nothing
prevents someone from attempting to re-use the function with a
potentially-nil argument though.

I don't know what makes more sense here: adding an assertion?  Handling
the nil case explicitly for robustness?

>> Subject: [PATCH 6/6] Simplify highlighting assertions
>>
>> * test/lisp/dired-aux-tests.el (dired-test--check-highlighting):
>> New function.
>> (dired-test-highlight-metachar): Use it.
>
> Will this simplification hinder debugging of test failures?  I don't
> have an opinion on the proposed change, it's just something to consider.

Mmm.  Since the assertion that fails is now nested in a more generic
function, the report shown in the ERT-Results buffer might be somewhat
less informative; one has to bring up the backtrace to understand the
context.

I could try my hand at an ERT explainer for these assertions.  Or we
could just drop the 6th patch…  I do find the tests easier to read and
write with it though.


PS: Looking at this made me realize that patch #5 was borked (missed a
parenthesis in dired-test-highlight-metachar, so the tests just plain
wouldn't run).  Here is the patch series with patches #5 and #6 fixed.

The squashed patch[1] remains the same.

[0001-Preserve-text-properties-in-y-or-n-p-prompts.patch (text/x-diff, attachment)]

[0002-Tweak-dired-warning-about-wildcard-characters.patch (text/x-diff, attachment)]

[0003-Dedup-dired-aux-isolated-char-searching-Bug-35564.patch (text/x-diff, attachment)]

[0004-fixup-Dedup-dired-aux-isolated-char-searching-Bug-35.patch (text/x-diff, attachment)]

[0005-Add-markers-below-non-isolated-chars-in-dired-prompt.patch (text/x-diff, attachment)]

[0006-Simplify-highlighting-assertions.patch (text/x-diff, attachment)]

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

Thank you for your review.


[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28969#19

Previous Next

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