[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 <kevin.legouguec@gmail.com> writes:
> "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?
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
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Kévin Le Gouguec, 2019/07/03
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Kévin Le Gouguec, 2019/07/12
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Eli Zaretskii, 2019/07/27
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Kévin Le Gouguec, 2019/07/27
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Michael Heerdegen, 2019/07/27
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Michael Heerdegen, 2019/07/28
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Juri Linkov, 2019/07/29
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Kévin Le Gouguec, 2019/07/29
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Basil L. Contovounesios, 2019/07/27
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters, Kévin Le Gouguec, 2019/07/27
- bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters,
Basil L. Contovounesios <=