huijunchen9260 / dmenufm

A simple file manager using dmenu
GNU General Public License v3.0
227 stars 15 forks source link

Fix IFS messing with commands (FINALLY) #18

Closed camnw closed 4 years ago

camnw commented 4 years ago

OKAY! I believe this will solve both the space name #15 issue and the newly introduced unknown command cp -R issue by this commit #16 , do test to verify this but it seems to work for me when using dash. Also worth mentioning this will negate #17 unless further issues arise with this.

The issue was caused by me not realizing using IFS as new line would try to parse cp -R as a whole command (and thus ln -s), the more you know I guess, had no clue this would be a side effect of setting the IFS in the wrong position!

huijunchen9260 commented 4 years ago

Should we introduce this to all the for loops in both DEL and TRH?

camnw commented 4 years ago

Yes, if you would like to do this that should work. The only important part with IFS is that going into the for loop it is set to just new line, so just make sure it is in a similar spot as where I moved it as seen above (right after for loop starts)

camnw commented 4 years ago

Try with this newest commit.

huijunchen9260 commented 4 years ago

Wow! This works as a charm! Although I still don't know how IFS works LOL Do you have any recommended material that I can learn about this part of POSIX shell / bash?

camnw commented 4 years ago

The man page explanation I found was just very basic, I found this page which I feel explains it well https://mywiki.wooledge.org/IFS . Also after reading this, I realized simply using unset $IFS is alot cleaner than how I thought to do it by storing the oldIFS variable, so it may be worth changing this to use unset IFS to reset it.

huijunchen9260 commented 4 years ago

Would you mind providing an example in dmenufm?

camnw commented 4 years ago

@huijunchen9260 Not strictly dmenufm, but this is using dmenufm variables and problems we have faced.

Alright, so lets pretend you selected four files new file newer file emi says hi working_file.

$ selected="/home/cameron/emi says hi
> /home/cameron/new file
> /home/cameron/newer file
> /home/cameron/working_file"

So, everything seems fine. Now let us try to iterate over a variable with a for loop:

$ for file in $selected; do
> echo $file
> done
/home/cameron/emi
says
hi
/home/cameron/new
file
/home/cameron/newer
file
/home/cameron/working_file

You will see the for loop separated the text when it runs into a new line or a space. So you end up with the spaced name files getting cut off. Now, let us set the IFS to JUST a new line character (by inserting a new line between its quotes)

$ IFS="
> "
$ for file in $selected; do
> echo $file
> done
/home/cameron/emi says hi
/home/cameron/new file
/home/cameron/newer file
/home/cameron/working_file

You will now see that this properly splits at new lines and NOT spaces. Now, I will explain what the cp -R issue is about.

$ touch newfile
$ cp -R newfile newerfile
$ cmd="cp -R"
$ unset IFS # Setting IFS to its original values
$ $cmd newerfile newestfile
$ du newestfile # show that the file exists, thus copy worked
0       newestfile
$ IFS="
> "
$ $cmd newestfile errorfile
dash: 39: cp -R: not found

You will see that after I set $IFS to only split at new lines, it tries to treat the entirety of cp -R as one command, as it only thinks it should be splitting it if there is a new line. To show this even more we can then set our copy command to be a two liner!

$ cmd="cp
> -R"  # Set our cmd to be two lines
$ $cmd newestfile nonerrorfile
$ du nonerrorfile
0       nonerrorfile

This will now work because it splits the command into two parts, the cp and then the -r If you have anything else to ask feel free :)

huijunchen9260 commented 4 years ago

Wow! Such a clear explanation for IFS! Thank you again for teaching me such a basic yet important concept in shell scripting!