zsh-workers
 help / color / mirror / code / Atom feed
From: Peter Stephenson <Peter.Stephenson@csr.com>
To: zsh workers <zsh-workers@zsh.org>
Subject: Re: [RFC or so] Add HASH_LOOKUP option
Date: Mon, 23 Aug 2010 15:55:27 +0100	[thread overview]
Message-ID: <20100823155527.6e11c3bf@csr.com> (raw)
In-Reply-To: <AANLkTim0NXjt4hp-UV4PK_7wNS2+tvf8NOu9W8F9RUSD@mail.gmail.com>

On Mon, 23 Aug 2010 16:09:45 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> When this is unset, external commands are always resolved with a full
> path search, but still inserted into the hash for spell correction if
> those options are on.

Looks a reasonable thing to be able to do.  The word on the streets (for a
tiny, tiny subset of streets) has for some time been that modern operating
systems do their own optimisation of path lookup such that hashing in the
shell doesn't gain you very much.  The main reason we haven't done anything
about it is that we need the hash for other reasons.  You've sidestepped
that simply by always searching and still hashing it (depending on the
existing options).  That's a very minor performance hit beyond the path
search, so I can't see any objection.

Telling people to turn it off is probably the big problem.  I'd be
sympathetic to turning it off by default if there's enough evidence the
performance hit is minor, but that's all rather system-specific, so don't
hold your breath.  I'd be wary of turning it off by default on Cygwin with
paths to remote shares, for example.

It would be useful if you could write some tests to go in
Test/E01options.ztst.  Basically you just need to test the right version
of a command is found with it on and off.  It would be good to do a test
along the lines of

  mkdir dir1 dir2
  mkcmd() {
    print "#!/bin/sh\necho Command $1" >dir$1/cmd
    chmod +x dir$1/cmd
  }
  mkcmd 2
  for opt in on off on off; do
    if [[ $opt = on ]]; then
      setopt hashlookup
    else
      unsetopt hashlookup
    fi
    rm -rf dir1/cmd
    (path=($PWD/dir1 $PWD/dir2)
     cmd
     mkcmd 1
     cmd)
  done

and verify that when the option is off the second cmd prints
"Command 1" each time round and when it's on it doesn't.  (I haven't tested
this code.)

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom


  reply	other threads:[~2010-08-23 14:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-23 14:09 Mikael Magnusson
2010-08-23 14:55 ` Peter Stephenson [this message]
2010-08-23 17:46 ` Bart Schaefer
2010-08-23 18:25   ` Mikael Magnusson
2010-08-24  8:37     ` Bart Schaefer
2011-06-02 15:11   ` Mikael Magnusson

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=20100823155527.6e11c3bf@csr.com \
    --to=peter.stephenson@csr.com \
    --cc=zsh-workers@zsh.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.vuxu.org/mirror/zsh/

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 NNTP newsgroup(s).