[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Basil L. Contovounesios
Subject: bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters
Date: Sun, 28 Jul 2019 00:41:20 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux)

Kévin Le Gouguec <address@hidden> writes:

> "Basil L. Contovounesios" <address@hidden> 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,


reply via email to

[Prev in Thread] Current Thread [Next in Thread]