Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bugs when searching for multiple words or for spaces #36

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

subraizada3
Copy link

@subraizada3 subraizada3 commented Jun 14, 2019

This is an extension of #32

Examples contain output from running spot inside its repository directory.

The change on line 121 is required to suppress an error message printed when searching for multiple words:

$ spot force color # old version
./spot.sh: line 121: [: force: binary operator expected

./spot.sh:
149:# add force colors

$ spot force color # new version
./spot.sh:
149:# add force colors

On line 163, replacing "$@" with "`echo $@`" is required for multiple word search to work properly:

$ spot force color # old version, in the results only 'force' is highlighted
grep: color: No such file or directory

./spot.sh:
45:    -s, --sensitive         Force case sensitive search.

./spot.sh:
46:    -i, --insensitive       Force case insensitive search.

./spot.sh:
48:    -C, --no-colors         Force avoid colors.

./spot.sh:
149:# add force colors


$ spot force color # new version, in the results the entire 'force color' phrase is highlighted
./spot.sh:
149:# add force colors

On that line, adding quotation marks around the $@ in the echo is required to make searching for an empty string to work:

$ spot "                " # version from other PR: "`echo $@`"
   [matches every single line in every file in the directory]

$ spot "                " # new version: "`echo \"$@\"`"
   [matches a single line with many spaces in it; the spaces are highlighted]
./spot.sh:
54:    --                      End of options

Finally, this fixes an error when trying to search for slashes:

$ spot //   # old version
(no search term. `spot -h` for usage)

$ spot //   # new version

./README.md:
19:![](https://cldup.com/TiVORMfp77-1200x1200.png)

# more results omitted

./spot.sh:
115:    dir=${1/%\//}

./spot.1:
2:.\" http://github.com/kapouer/ronnjs/

On line 113, the '//' search term is detected as a directory (at least on bash, '//', '///', etc. are interpreted as just '/' and so pass the -d test on line 114). The additional regex test on 113 verifies that there is at least one non-'/' character before counting it as a manually-specified search directory. The only issue is that this makes it impossible to do a search on the entire filesystem by doing spot / searchTerm - the '/' is not detected as a directory since it does not contain a non-'/' character. The two workarounds are to either:

  • remove the second test on line 113 temporarily, or
  • cd /; spot ./ searchTerm

@rauchg
Copy link
Owner

rauchg commented Jul 13, 2019

That's amazing!

@rauchg
Copy link
Owner

rauchg commented Jul 13, 2019

Do you think you could write a quick (bash of course ;)) script that runs some tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants