Problems with my shell script... (grep)

Question

I'm sorry to be posting about another problem with the same script that I was working on yesterday.

Originally I had a problem with it cding to paths with spaces, though that is fixed now. The problem is that if a 3rd argument is given to the script, it is to search for it within the files it found earlier and then print the files location as well as the line number the term was found on.

For some reason grep isn't liking paths to files that contain spaces (again right? -.-) even though I have double quoted the variable I am greping.

Does anyone have any ideas about how to fix it?

#!/bin/bash path = $1 #1st arg is the path to be searched regex = $2 #2nd arg is a regular expression searchTerm = $3 #3rd arg is an optional search term startDir = `pwd` #Stores the starting path getDirs() { #Function to get the directories for i in "$1" ; do if [ -d "$i" ]; then echo "$i" >> temp.txt getDirs "$i" fi done } getFiles() { # Function to get files matching the regex while IFS= read -r path; do # While there is a line, read it, backslash is not a delimiter cd "$path" temp=`ls -1 | grep "$regex"` #List the contents of the dir. Store only files that match the regex for j in $temp do echo "$path/$j" # For every file stored, print its location done cd $startDir done < temp.txt # Read from temp.txt } searchFiles() { # Function to search within files for a in $output1 # For every file found do out=`grep -n "$searchTerm" "$a" | cut -d: -f 1` # Find the line numbers in which it is present, stop showing after 1st : for i in $out # For every line found do echo "$a: line $i" # Print the file location, and the line numbers of the terms done done } numArgs=$# echo "$path" >> temp.txt getDirs $path # Getting directories to search output1=`getFiles` cd $startDir if [ $numArgs == 3 ] # If a search term is specified then searchFiles # Then search the files for it else echo "$output1" # Otherwise, just print the location of the files fi rm temp.txt # Removing temporary files exit 0

Please post your code here, not via a link. And if it seems too long, then it is! You'd need to reduce it to a minimal test-case. — Oliver Charlesworth
– Oliver Charlesworth, Commented Jan 6, 2013 at 13:45
I'm pretty sure the issue is with the searchFiles function, that's where my grep is — Dom Brown
– Dom Brown, Commented Jan 6, 2013 at 13:54
Are you sure the issue is with grep? Looking at the code, I'd consider for a in $output1 as a possible issue, as that will likely break $output1 by spaces. What happens if you set an empty IFS before the for loop? — njsg
– njsg, Commented Jan 6, 2013 at 14:06
I tried putting double quotes around $output1 and seemed to not work, if anything it made it worse — Dom Brown
– Dom Brown, Commented Jan 6, 2013 at 14:12

Ed Morton · Accepted Answer · 2013-01-06 17:22:09Z

Your script has a LOT of problems including unquoted and incorrectly quoted variables. Here's how your getFiles function need to be written at a minimum (there are other issues like whether grep is really necessary and use of echo but I'm not touching those so this highlights the serious problems):

getFiles() { # Function to get files matching the regex while IFS= read -r path; do # While there is a line, read it, backslash is not a delimiter if cd "$path"; then oIFS="$IFS" # save then reset and restore IFS to avoid work splitting on spaces, except newlines. IFS=$'\n' tempA=( $(ls -1 | grep "$regex") ) #List the contents of the dir. Store only files that match the regex IFS="$oIFS" for j in "${tempA[@]}" do echo "$path/$j" # For every file stored, print its location done cd "$startDir" fi done < temp.txt # Read from temp.txt }

Note that "temp" is now an array, not a string, so you can access the file names it contains one at a time and still have each of them quoted. I just renamed it tempA to make it obvious that it's an Array.

So, update your script to use arrays instead of strings to hold your file names as demonstrated above, get rid of the spaces around the assignments, quote all of your variables, use $(...) instead of backticks, and change grep -n "$searchTerm" "$a" | cut -d: -f 1 to awk -v st="$searchTerm" '$0~st{print NR}' "$a" then repost if you still have problems.

Thank you for the help, I'm relatively new to Bash having just been introduced to it at University and left to pretty much teach it to ourselves so I apologise for anything horribly bad :) I've managed to get my original script to work by manipulating spaces in the paths using sed. Though I will try to implement the improvements you've given me anyway, I need the practice! Thanks for the advice :)
Do yourself a favor and do NOT do it the way you describe as that's like owning a bicycle and carrying it everywhere you walk - you can do it but people will look at you kinda funny! The resulting script also cannot help being riddled with bugs and difficult to enhance/maintain in future as it's using tools that are simply not designed for the job and guess what the response will be here if you post a question asking for help.
Note, your example code is still broken here for files containing whitespace. The tempA=( $(ls -1 | grep ...) ) expression will undergo word splitting.
@JoshCartwright yes, you're right. I'd set IFS=$'\n' before the operation - any better suggestion?
The solution is to avoid the use of the $(ls -1 | grep ...) construct altogether. It is not possible to use properly with files with embedded whitespace. If it is possible to assume GNU find, I would suggest making use of find $dir -regex .... If GNU find cannot be assumed, a bash loop over a glob expansion might work coupled with [[ a =~ b ]] (assuming a newer bash) or expr a : b.

Collectives™ on Stack Overflow

Problems with my shell script... (grep)

1 Answer 1

10 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

10 Comments

Related