bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters

[Kévin Le Gouguec]

"Basil L. Contovounesios" <contovob@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
Description: Text Data

0002-Tweak-dired-warning-about-wildcard-characters.patch
Description: Text Data

0003-Dedup-dired-aux-isolated-char-searching-Bug-35564.patch
Description: Text Data

0004-fixup-Dedup-dired-aux-isolated-char-searching-Bug-35.patch
Description: Text Data

0005-Add-markers-below-non-isolated-chars-in-dired-prompt.patch
Description: Text Data

0006-Simplify-highlighting-assertions.patch
Description: Text Data

Thank you for your review.


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