all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "Drew Adams" <drew.adams@oracle.com>
To: "'Stefan Monnier'" <monnier@iro.umontreal.ca>
Cc: 'Eli Zaretskii' <eliz@gnu.org>, emacs-devel@gnu.org
Subject: RE: next emacs version?
Date: Sat, 20 Mar 2010 16:09:23 -0700	[thread overview]
Message-ID: <A0F8070B5E0947C2AEBA46B5152946E3@us.oracle.com> (raw)
In-Reply-To: <jwv634qacfk.fsf-monnier+emacs@gnu.org>

>> 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	other threads:[~2010-03-20 23:09 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-19 11:23 next emacs version? Drew Adams
2010-03-19 13:22 ` Eli Zaretskii
2010-03-19 17:29   ` Drew Adams
2010-03-19 18:09     ` Eli Zaretskii
2010-03-19 18:46       ` Drew Adams
2010-03-19 19:02         ` Eli Zaretskii
2010-03-19 20:02           ` Drew Adams
2010-03-19 21:15             ` Eli Zaretskii
2010-03-19 21:23               ` Drew Adams
2010-03-20  2:35                 ` Ken Raeburn
2010-03-20  2:39                   ` Lennart Borgman
2010-03-20  3:42                     ` Óscar Fuentes
2010-03-20 15:51                       ` Lennart Borgman
2010-03-20  5:31                     ` Ken Raeburn
2010-03-23  2:05                     ` Stephen J. Turnbull
2010-03-20  3:38                   ` Drew Adams
2010-03-20  5:31                     ` Ken Raeburn
2010-03-20  6:51                       ` Drew Adams
2010-03-20  5:31                     ` Ken Raeburn
2010-03-20  6:51                       ` Drew Adams
2010-03-23  2:34                     ` Stephen J. Turnbull
2010-03-23  5:01                       ` Miles Bader
2010-03-23  5:39                       ` Drew Adams
2010-03-20  3:51                 ` Jason Rumney
2010-03-20  6:47                   ` Drew Adams
2010-03-20  8:39                   ` Eli Zaretskii
2010-03-20 14:58                     ` Drew Adams
2010-03-20 16:22                       ` Eli Zaretskii
2010-03-20  8:11                 ` Eli Zaretskii
2010-03-19 20:55           ` Stefan Monnier
2010-03-19 21:16             ` Drew Adams
2010-03-20 19:10               ` Stefan Monnier
2010-03-20 20:29                 ` Drew Adams
2010-03-20 21:53                   ` Stefan Monnier
2010-03-20 23:09                     ` Drew Adams [this message]
2010-03-20 23:26                       ` Drew Adams
2010-03-22  1:22                       ` Stefan Monnier
2010-03-22  7:22                         ` Drew Adams
2010-03-22 13:52                           ` Stefan Monnier
2010-03-21 21:34     ` Thien-Thi Nguyen
2010-03-21 23:20       ` Drew Adams
2010-03-19 14:52 ` Chong Yidong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=A0F8070B5E0947C2AEBA46B5152946E3@us.oracle.com \
    --to=drew.adams@oracle.com \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.