Skip to content

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

Closed
subraizada3 wants to merge 2 commits intorauchg:masterfrom
subraizada3:master
Closed

fix bugs when searching for multiple words or for spaces#36
subraizada3 wants to merge 2 commits intorauchg:masterfrom
subraizada3:master

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?

@subraizada3 subraizada3 closed this by deleting the head repository Oct 3, 2024
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.

2 participants