emacs-devel
[Top][All Lists]
Advanced

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

RE: next emacs version?


From: Drew Adams
Subject: RE: next emacs version?
Date: Sat, 20 Mar 2010 16:09:23 -0700

>> That would involve my code with the bug-fix code (logic).
>
> Not at all.  It simply amounts to writing the unit-test that checks
> whether the bug is fixed: (string-match <regexp> <problematic-dired-line>)
> It's simple and robust.

That's precisely what I mean by involving my code with the bug-fix code: adding
a unit-test for an unrelated bug fix in the middle of font-lock keywords. Extra
code-coupling.

*Why* the regexp is the way it is (better support for file names that include
ISO dates, or whatever) is unimportant for the font-lock code logic. That code
should not care how the regexp does its job of matching a date and file-name or
which formats are currently supported. All that the font-lock code cares about
is picking up the correct regexp group for the date+time info.

(I see, BTW, that I should not have mentioned condition-case in my mail - dunno
what I was thinking there.)

This is presumably what you're suggesting for the font-lock keyword entry:

(list dired-move-to-filename-regexp
      (if (string-match           ; Date/time
            dired-move-to-filename-regexp
            <problematic-dired-line>)
          (list 2 'diredp-date-time t t)
        (list 1 'diredp-date-time t t))
      (list "\\(.+\\)$" nil nil   ; File w/o suffix
            (list 0 diredp-file-name 'keep t)))

It's not obvious to me what the <problematic-dired-line> string should be. I
tried this, based on the bug #5597 report, which says that a file named
"~/2010-02-18\ foo" causes the unfixed (i.e. old) regexp not to match:

 "-rw-rw-rw- 1 10 2010-04-01 2010-02-18 foo"

But that in fact _matches_ the old regexp as well, i.e., in versions before the
fix (Emacs 20.7, 21.3.1, 22.3, 23.1, 23.1.92). And of course there's a
possibility that the regexp might match even in the bugged case, but match in
the wrong way. None of that is called out in the bug history, so it would
require further investigation (with an extremely hairy regexp).

So you can see one manifestation of the added complication already: getting the
unit test right.

And as I said, I cannot even reproduce the bug itself on Windows, starting with
emacs -Q, in any release that I have. That seems to indicate that the bug (and
hence your approach) might be platform-specific. (Or perhaps the bug report and
fix description are not clear.)

Whereas a simple `emacs-version' test has to work (since the fix is in only in
newer versions), code that addresses the bug case directly will do the wrong
thing on any platforms where the bug was not manifested. The buggy file name can
only serve to identify which regexp is which if in fact one of the regexps (the
old one) is buggy on the current platform.

To fix it further (assuming such platform-dependence), I would need to then
investigate all platforms and add platform tests to the embedded unit-test code.
And if the problem was not that the regexp didn't match but that it matched in
the wrong way...

Such an approach is not clean and sound, IMO. It means unnecessary
code-coupling, which is unhealthy: mixing in bug-case unit-test code with the
font-lock code, which is logically unrelated.

Coding that way and maintaining the code then requires understanding the bug and
the fix. At the very least, a WTF comment would need to be added so that anyone
reading the code would have some idea what was going on.

Again, if you think I misunderstand, please clarify. Thx.





reply via email to

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