unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Dave Abrahams <dave@boostpro.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 12351@debbugs.gnu.org
Subject: bug#12351: 24.1; parse-colon-path turns empty paths into nil
Date: Sun, 30 Dec 2012 15:37:16 -0500	[thread overview]
Message-ID: <m2k3rz9szn.fsf@cube.luannocracy.com> (raw)
In-Reply-To: <83licfjnny.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 30 Dec 2012 22:22:09 +0200")


on Sun Dec 30 2012, Eli Zaretskii <eliz-AT-gnu.org> wrote:

>> From: Dave Abrahams <dave@boostpro.com>
>> Date: Sun, 30 Dec 2012 14:53:44 -0500
>> Cc: 12351@debbugs.gnu.org
>> 
>> > Obviously we need the nils to remain, so I will put them back and just
>> > mention that empty elements return nil. It's not worth handling the
>> > minor aesthetic annoyance of (nil nil) specially.
>> 
>> FWIW, I disagree. IMO you should at least consider fixing eshell and any
>> other things that break because of this change.  This discontinuity in
>> behavior is not merely aesthetic; it makes parse-colon-path difficult to
>> use correctly and leads to hard-to-find bugs in any code that fails to
>> account for the possible nils.
>
> This whole discussion is rather futile, unless the opinions are also
> backed up by real-life use cases.  Can you tell why the previous
> behavior made parse-colon-path difficult to use, and in what
> situations?

Instead of recording that complex situation when I encountered the bug I
helpfully (!) recorded a reduced reproducible example that stripped away
the use case, which I didn't remember... but I even went the extra mile
to reconstruct it.  For example, look at 

http://edward.oconnor.cx/elisp/osx-plist.el

The following function is buggy because of the original bug:

--8<---------------cut here---------------start------------->8---
(defun osx-plist-update-exec-path ()
  "Update `exec-path' from the PATH environment variable."
  (let ((path (getenv "PATH")))
    (mapc (lambda (dir)
            (add-to-list 'exec-path dir))
          (parse-colon-path path)))
  exec-path)
--8<---------------cut here---------------end--------------->8---

I had to replace it in my local installation as follows:

--8<---------------cut here---------------start------------->8---
  (defun osx-plist-update-exec-path ()
    "Update `exec-path' from the PATH environment variable."
    (let ((path (delq nil (parse-colon-path (getenv "PATH")))))
      (setq exec-path
            (dolist (dir exec-path path)
              (add-to-list 'path (file-name-as-directory dir) :append)))))
--8<---------------cut here---------------end--------------->8---

If you go looking for instances of parse-colon-path I'm sure you'll find
hundreds of other places where the use was tailored to the documented
behavior of parse-colon-path rather than the specific oddball behavior
that was actually implemented.  I found at least one in my own code just
now.

IMO, though, you should actually be able to understand this one without
any examples.  Any discontinuity in behavior means the client needs to
write special-case code to handle that special-case behavior.  For one
or two clients it may be that the special-case behavior matches just
what they need, but in general that's highly unlikely.  Combine this
with the fact that the uniform behavior has been documented for years,
and that the inputs that trigger the non-uniformity are rare, and you
can be pretty confident that more code has been written to the uniform
specification.

-- 
Dave Abrahams
BoostPro Computing                  Software Development        Training
http://www.boostpro.com             Clang/LLVM/EDG Compilers  C++  Boost





  reply	other threads:[~2012-12-30 20:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-04 18:58 bug#12351: 24.1; parse-colon-path turns empty paths into nil Dave Abrahams
2012-09-16 15:57 ` Chong Yidong
2012-12-30 18:52 ` Glenn Morris
2012-12-30 19:53   ` Dave Abrahams
2012-12-30 20:22     ` Eli Zaretskii
2012-12-30 20:37       ` Dave Abrahams [this message]
2012-12-30 22:08         ` Andreas Schwab
2012-12-31  3:08           ` Dave Abrahams
2012-12-31  7:14             ` Glenn Morris
2012-12-31 12:59               ` Dave Abrahams
2012-12-31 10:07             ` Andreas Schwab
2012-12-31  1:40         ` Wolfgang Jenkner
2012-12-31  3:11           ` Dave Abrahams
2012-12-31  6:56             ` Glenn Morris
2012-12-31  7:07               ` Glenn Morris
2012-12-31 12:58                 ` Dave Abrahams
2012-12-31 16:10             ` Wolfgang Jenkner

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

  List information: https://www.gnu.org/software/emacs/

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

  git send-email \
    --in-reply-to=m2k3rz9szn.fsf@cube.luannocracy.com \
    --to=dave@boostpro.com \
    --cc=12351@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    /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 public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).