Skip to content

Commit

Permalink
For #115: Update link file with newline test.
Browse files Browse the repository at this point in the history
 - IFS manipulation did not appear to be doing anything helpful, so I
   removed it.
 - Linking files with a newline is not actually fixed, but the
   behavior did change slightly, so I updated the test.
  • Loading branch information
dougborg committed Oct 14, 2014
1 parent 4258c16 commit ed0b568
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 11 deletions.
9 changes: 3 additions & 6 deletions lib/commands/link.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,13 @@ function symlink {
ignore 'ignored' "$castle"
return $EX_SUCCESS
fi
oldIFS=$IFS
IFS=$'\n'

This comment has been minimized.

Copy link
@andsens

andsens Nov 10, 2014

Owner

Hehe, yeah. Turns out the IFS change actually had a purpose (#123). It's my fault, I should've checked the PR more closely and most of all not refactored out the code that tested this, when I moved the testing framework to bats.

This comment has been minimized.

Copy link
@dougborg

dougborg Nov 10, 2014

Author Contributor

Oh noes! Sorry about that.

Another way to go for this loop would be to use a while loop + read since it is line-based anyway and doesn't block waiting for get_repo_files to complete.

This comment has been minimized.

Copy link
@andsens

andsens Nov 10, 2014

Owner

Nope, can't do: acc8c2d
There are prompts inside the while loop that read from stdin, they would be spammed with the output of get_repo_files :-)
However: One could maybe use a third fd for the loop substitution using mkfifo or something similar...

This comment has been minimized.

Copy link
@dougborg

dougborg Nov 10, 2014

Author Contributor

I actually think I had it set up that way at one point, but went back to the for loop for some reason or another in the course of messing around. The safest way to do with a while loop it is to have the get_repo_files output it to another stream (i.e. not std err / std out) and read it in from that stream:

    while read filename <&3; do
      ...
    done 3< <(get_repo_files "$repo/home")

This comment has been minimized.

Copy link
@dougborg

dougborg Nov 10, 2014

Author Contributor

Yep! that was the reason. I think using file descriptor 3 will work though. I hadn't learned that trick yet :-)

This comment has been minimized.

Copy link
@andsens

andsens Nov 10, 2014

Owner

Awesome! That's definitely the way to go. This would allow us to link files with newlines in their name - now even maniacs can use homeshick, wohoo!

This comment has been minimized.

Copy link
@dougborg

dougborg Nov 10, 2014

Author Contributor

LMFAO!

+1 for maniacs using homeshick!

This comment has been minimized.

Copy link
@andsens

andsens Nov 10, 2014

Owner

Haha, that was easy! Done: 9f1c91c
Note that the bug with the prompt getting input from the process substitution already has a test, so I only added a new one where the filename has a bell character and a smiley face.

for filename in $(get_repo_files $repo/home); do
for filename in $(get_repo_files "$repo/home"); do
remote="$repo/home/$filename"
IFS=$oldIFS
local=$HOME/$filename
local="$HOME/$filename"

if [[ -e $local || -L $local ]]; then
# $local exists (but may be a dead symlink)
if [[ -L $local && $(readlink "$local") == $remote ]]; then
if [[ -L $local && $(readlink "$local") == "$remote" ]]; then
# $local symlinks to $remote.
if [[ -d $remote && ! -L $remote ]]; then
# If $remote is a directory -> legacy handling.
Expand Down
9 changes: 4 additions & 5 deletions test/suites/link.bats
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,13 @@ EOF

@test 'fail when linking file with newline' {
castle 'rc-files'
touch "$HOMESICK/repos/rc-files/home/filename
test_filename="filename
newline"
touch "$HOMESICK/repos/rc-files/home/$test_filename"
commit_repo_state $HOMESICK/repos/rc-files
$HOMESHICK_FN --batch link rc-files
[ -L "$HOME/\"filename" ]
[ -L "$HOME/newline\"" ]
is_symlink $HOMESICK/repos/rc-files/home/\"filename $HOME/\"filename
is_symlink $HOMESICK/repos/rc-files/home/newline\" $HOME/newline\"
[ -L "$HOME/\"filenamennewline\"" ]
is_symlink "$HOMESICK/repos/rc-files/home/\"filenamennewline\"" "$HOME/\"filenamennewline\""
}

@test 'files ignored by git should not be linked' {
Expand Down

0 comments on commit ed0b568

Please sign in to comment.