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

Previous Next

Full log

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

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

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> "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?

I think it's fine the way it is, though a docstring/comment never hurts.
I was just checking.

>>> 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.

That's good enough for me,

-- 
Basil

Previous Next

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