don't add --exclude-dir to GREP_OPTIONS on FreeBSD

[kemko]

$ grep --exclude-dir='.svn' test ./*
grep: unrecognized option `--exclude-dir=.svn'
Usage: grep [OPTION]... PATTERN [FILE]...
Try `grep --help' for more information.
FAIL: 2

[ncanceill]

Every body should have --exclude-dir for grep.

You seem to be running an old/odd version of grep. You need GNU grep version 2.5.2 or later [ref].

[kemko]

$ uname -a
FreeBSD hostname 10.0-RELEASE FreeBSD 10.0-RELEASE #0 r260789: Thu Jan 16 22:34:59 UTC 2014     root@snap.freebsd.org:/usr/obj/usr/src/sys/GENERIC  amd64
$ grep --version
grep (GNU grep) 2.5.1-FreeBSD
$ grep --exclude-dir
grep: unrecognized option `--exclude-dir=.cvs'

perhaps in ports have a more recent version of the grep, but in basic installation only this.

[nopnop]

Same problem with OSx 10.7 (GNU grep v2.5.1)

Fixed using with brew (GNU grep v2.18):

brew install grep
ln -s /usr/local/opt/grep/bin/ggrep /usr/local/bin/grep

[ncanceill]

@kemko You confirm my diagnostic: you need grep 2.5.2 or later, and you have 2.5.1 so you have to upgrade

[kemko]

@ncanceill Version 2.5.2 can be obtained only by setting its from ports (analogue to brew on OSx). If that means - no problem. But I was hoping that OMZ can work on supported systems without installing additional software.

[mcornella]

It seems reasonable to only use --exclude-dir if grep supports it, though I disagree with the solution provided. I'd guess that grep --exclude-dir=sth will return a non-zero exit code if there isn't that parameter available.

[ncanceill]

Version 2.5.1 is nine years old! Sorry the official FreeBSD repo is so old. Anyway, I would definitely recommend upgrading via ports even without OMZ requiring it.

[donnex]

Oh my zsh broke my grep on all my systems too, FreeBSD

[mcornella]

On the other hand, as @ncanceill says it's in your best interest to upgrade your grep and you can do that easily with ports (it wouldn't require you downloading and compiling grep). Otherwise you can always set GREP_OPTIONS in your .zshrc.
If there is a significant amount of users that complain about this, the change should be made, but not with the current solution.

[julitrows]

On OS X 1.7.5, after upgrading grep to 2.18 via homebrew, and doing upgrade_oh_my_zsh, when starting a new terminal session, the error message still appears(notice the first lines below), but grep works and recognizes the option:

Last login: Mon Mar 24 13:26:13 on ttys001
grep: unrecognized option `--exclude-dir=.cvs'
Usage: grep [OPTION]... PATTERN [FILE]...
Try `grep --help' for more information.
➜  bin git:(master) grep --version
grep (GNU grep) 2.18
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Mike Haertel and others, see <http://git.sv.gnu.org/cgit/grep.git/tree/AUTHORS>.
➜  bin git:(master) ls -lha | grep "hi" --exclude-dir=.cvs
➜  bin git:(master) cd ~
➜  ~  ls -lha | grep "hi" --exclude-dir=.cvs
-rw-------    1 julio  staff   8,8K 24 mar 13:22 .bash_history
-rw-r--r--    1 julio  staff    52B  9 ene 14:27 .guard_history
-rw-r--r--    1 julio  staff   2,5K 21 mar 20:37 .irb-history
-rw-------    1 julio  staff     0B 13 mar 10:53 .mysql_history
-rw-------    1 julio  staff    65B  6 ene 11:58 .psql_history
-rw-r--r--    1 julio  staff   6,7K  5 mar 15:29 .rdebug_hist
-rw-------    1 julio  staff   173K 24 mar 13:28 .zsh_history
-rw-r--r--    1 julio  staff   2,1K 24 mar 10:11 yankring_history_v2.txt

Is it necessary to perform some other step to remove this message?

[kemko]

@mcornella I have not yet needed to use new options like --exclude-dir (maybe because I did not know about them), so update grep previously did not make sense for me. As you can see, I am not alone. In addition, add a workaround to .zshrc not so difficult, but if I have not used the OMZ before at all - would not like to start working with him to find solutions to fix problems.

The first patch was made in haste. Now slightly rewrited it for cases where the system has a grep installed from ports.

It isn't big deal to copy some workaround for this issue into my own .zshrc and forget about it, but I guess I did not mention that installation from ports is also used grep for some checks, so if grep was broken through unsupported parameters in GREP_OPTIONS - and you can not properly upgrade to more recent version (unless you'll be use pkg_add or execute unset GREP_OPTIONS).

[kemko]

@ncanceill Is that better?

upd: Answer himself. Poor checked on Linux, this code says that version 2.14 is less than 2.5.2

[ncanceill]

Yes you have switched the arguments: it should be

is-at-least 2.5.2 $GREP_VERSION

Also, maybe you can add a comment to explain that --exclude-dir requires version 2.5.2 or later?

[neilalbrock]

This worked for me on OSX 10.7.5:

brew install --default-names grep

The default brew install of grep prefixes the command with g, so the problem doesn't go away unless you force the name to the standard via the --default-names option.

Hope that helps.

[kemko]

@ncanceill Tried to correct all the flaws.

[mcornella]

That worked perfectly. Nice job! 👍

[ncanceill]

Looks good to me too! 👍

[jeroenh]

Wow, this one scared the bejeezus out of me, I thought my box was rooted when all of a sudden grep started spewing errors. Good thing I found this just in time before I completely reinstalled my box.

[kgadek]

Those patches worked for me (OS X 10.6.8).

[kriogenx0]

GREP_VERSION=`grep -V | grep GNU | awk '{print $4}'`

if [[ $GREP_VERSION > 2.5.2 ]]; then
  for PATTERN in .cvs .git .hg .svn; do
      GREP_OPTIONS+="--exclude-dir=$PATTERN "
  done
fi

[ncanceill]

Hi @kriogenx0, thank you for contributing. Nonetheless, I am afraid you are wrong. In the end, I fail to see what you seek to improve in this PR with your comment.

Mistake 1

`grep -V | grep GNU | awk '{print $4}'`

This is not enough: grep -V may output multiple lines, one of them including the phrase "GNU GPL". Best thing to do is pipe grep -V to head -1 like it is done in the current PR.
Moreover, this does not strip the build ID at the end of the version number. You can pipe the output to cut -f1 -d '-' in order to do that.

Mistake 2

if [[ $GREP_VERSION > 2.5.2 ]]

This does not work: it does alphanumerical ordering, which is not what is needed. For instance, 2.45.2 will be considered lower than 2.5.2. This is why the current PR uses is-at-least.

[Miista]

This fixes the problem for me! Running OS X 10.6.8

[clemensg]

Nice version parsing! Fixes the problem for me.
But what about using --exclude on grep versions below 2.5.3 and --exclude-dir if equal or above?

[ncanceill]

@clemensg

Is the --exclude option available on all versions below 2.5.3? and all flavors? (meaning GNU/DNS/etc.)

If you can confirm at least a version range, it could be included.

[Fabryz]

I have just updated oh-my-zsh to the last version and get that error too on MacOSX 10.7.4

grep: unrecognized option `--exclude-dir=.cvs'

I don't get if i need to manually modify something or if the patch will be rolled out on the next release

[labeneator]

Suggested patch:

...
# --exclude-dir is only available on 2.5.3 and later versions of grep
if grep --exclude-dir=.svn  2>&1 >/dev/null;
then
    for PATTERN in .cvs .git .hg .svn; do
        GREP_OPTIONS+="--exclude-dir=$PATTERN "
    done
fi
...

[kemko]

@labeneator

kemko@work: ~ $ LC_ALL=C grep --exclude-dir=.svn Usage: grep [OPTION]... PATTERN [FILE]... Try 'grep --help' for more information. FAIL: 2

[clemensg]

Nice idea, first test if --exclude-dir works. If not, use --exclude. If this also fails, just proceed with empty GREP_OPTIONS. What do you think?

[labeneator]

@kemko
Does this work for you? You should get 'bad grep' based on the shell output of your last comment

grep --exclude-dir=.svn  >/dev/null 2>&1 && echo "good grep" || echo "bad grep"

[kemko]

@labeneator my grep is "bad", but it's 2.14 and support --exclude-dir. grep exit with non-zero code if search pattern is not provided (see grep --help). I think something like grep --exclude-dir=.csv grep $0 > /dev/null 2>&1 or echo test | grep --exclude-dir=.csv test > /dev/null 2>&1 will work fine.

[mcornella]

@labeneator you need to provide PATTERN and an input (so a FILE argument or pass it from a pipe). I have --exclude-dir in my grep, as well as --exclude and your test fails because those arguments are not provided. The resulting test comand is a little ugly, but it won't work otherwise.

What @kemko said 😁

[labeneator]

@kemko, @mcornella, I see what you are saying. So the best test we have is something along the lines of:

if echo hello | grep --exclude-dir=.git -nire hello  >/dev/null 2>&1;
then
   echo "Set GREP_OPTIONS"
fi

Turns out that OSX Mavericks' grep which is version 2.5.1 supports --exclude-dir.

$ which grep
/usr/bin/grep

$ /usr/bin/grep --version
grep (BSD grep) 2.5.1-FreeBSD


$ git init /tmp/git-test
Initialized empty Git repository in /private/tmp/git-test/.git/
$ echo hello > /tmp/git-test/something
$ echo hello > /tmp/git-test/git-it

# Works as expected. .git is skipped.
$ grep --exclude-dir=.git -nire h  /tmp/git-test
/tmp/git-test/git-it:1:hello
/tmp/git-test/something:1:hello

$ grep  -nire h  /tmp/git-test | head -3
/tmp/git-test/.git/description:1:Unnamed repository; edit this file 'description' to name the repository.
/tmp/git-test/.git/HEAD:1:ref: refs/heads/master
/tmp/git-test/.git/hooks/applypatch-msg.sample:1:#!/bin/sh

$ GREP_OPTIONS="--exclude-dir=.git"
$ grep  -nire h  /tmp/git-test | head -3
/tmp/git-test/git-it:1:hello
/tmp/git-test/something:1:hello

[Fabryz]

@ncanceill It worked for me, thanks

[mcornella]

I have cleaned @labeneator's proposal a bit, so the whole file looks like this (I rearranged a couple of lines too to make it prettier)

#
# Color grep results
# Examples: http://rubyurl.com/ZXv
#

GREP_OPTIONS="--color=auto"

# avoid VCS folders (if the necessary grep flags are available)
grep-flag-available() {
    echo | grep $1 "" >/dev/null 2>&1
}
if grep-flag-available --exclude-dir=.cvs; then
    for PATTERN in .cvs .git .hg .svn; do
        GREP_OPTIONS+=" --exclude-dir=$PATTERN"
    done
elif grep-flag-available --exclude=.cvs; then
    for PATTERN in .cvs .git .hg .svn; do
        GREP_OPTIONS+=" --exclude=$PATTERN"
    done
fi
unfunction grep-flag-available

export GREP_OPTIONS="$GREP_OPTIONS"
export GREP_COLOR='1;32'

[clemensg]

@mcornella Nice 👍

[ncanceill]

👍 @mcornella's version

@kemko can you update the PR?

[labeneator]

Works for me too. Ship it.

             .
             _\____
            |_===__`.        ==/
            \/  '---"\ _ _ _ _/
      ______/_______/_|_|_|_|_|
    _|--------------------==."
     \____________________.'  LGB

[Stealthii]

Seems I was a bit late with submitting my patch (#2720) but @mcornella's works with --exclude as well. +1 for legacy support! Ship it.

[kemko]

@ncanceill done

[ncanceill]

Amazing! @robbyrussell this PR is so needed and ready for merge

[sc68cal]

Hitting this on OS X - please fix asap.

[clemensg]

@robbyrussell ?

[ncanceill]

Actually @kemko can you squash your commits to keep the history clean?

[kemko]

@ncanceill I tried. I hope I did everything right.
upd: hmmm. but it seems not. I do not mind if you commit this fix youself (in another PR for example). it may be months until understand git :)

for now i'm tried: git clone git@github.com:kemko/oh-my-zsh
git checkout -b patch1 origin/patch-1
git rebase -i HEAD~5
[after squash old commits]
git push -f

[clemensg]

Not exactly.. Maybe you were still on your patch-1 branch when you did something else, and then those three extra patches from two other people made their way into this PR. I suggest you reset to the time when you created this branch and redo the changes + a single commit, then force push the branch to your repo.

Otherwise, closing this PR and creating a new one from a clean branch with just that one exclude-dir commit might be easier ;)

[kemko]

think I figured out what was wrong. Is that better?

[kjelderg]

That looks right to me and a fresh pull works just swell. (freebsd 9.1) Thanks @kemko

[ncanceill]

Yes, that's cleaner, thanks!

[mcornella]

Perfect! 👍 Safe to merge @robbyrussell