Untracked files with spaces in the path do not show a preview with ga

[sandr01d]

Check list

• [X] I have read through the README
• [X] I have the latest version of forgit
• [X] I have searched through the existing issues

Environment info

• OS
  • [X] Linux
  • [ ] Mac OS X
  • [ ] Windows
  • [ ] Others:
• Shell
  • [X] bash
  • [X] zsh
  • [X] fish

Problem / Steps to reproduce

1. Create a new file in a git repository that has a space character in the name and some content.

   echo test > untracked\ file

2. Use ga on the newly created file

Expected result

A line containing the word test is shown as an addition in the preview.

Actual result

The preview is completely empty.

I'll look into this once I find the time.

[cjappl]

Can reproduce the issue on my side. Tried to fix it for a bit but didn't get anywhere interesting. I'll try again if I have time (and no one else beats me to it)

I fixed a similar issue in fish before the merge. You can see those two here, maybe that has a clue in it?

Issue against fish:

206

Code:

209

204

One of the things I did in #209 we may want to replicate is breaking apart the big sed commands into named variables/helper function. It would help with debugging and readability (as I find sed fairly unreadable)

function forgit::extract_file --argument-names 'path'
    set no_leading_whitespace (echo $path | sed 's/^[[:space:]]*//')
    set no_m_or_double_question (echo $no_leading_whitespace | cut -d ' ' -f 2-)
    set if_renamed (echo $no_m_or_double_question | sed 's/.* -> //')
    set no_quotes (echo $if_renamed | tr -d "\"")
    set no_leading_whitespace_again (echo $no_quotes | sed 's/^[[:space:]]*//')
    set no_spaces (echo $no_leading_whitespace_again | string escape --no-quoted)
    echo $no_spaces
end

[sandr01d]

I've found the root of the issue, but couldn't figure out a solution yet. The issue is in the string that is stored in the preview variable:

https://github.com/wfxr/forgit/blob/ffda73bac3a435a9bbc6f29f2fd98517fbe5d9db/bin/git-forgit#L155-L161

$file is used unquoted here, which results in word splitting when there is a space in the file name. Adding escaped double quotes like \"\$file\" can not fix this.

[sandr01d]

I found a working solution, but I'm not really happy with it:

    read -r -d '' preview <<EOF
        file=\$(echo {} | $extract)
        if (git status -s -- \"\$file\" | grep '^??') &> /dev/null; then
            git diff --color=always --no-index -- /dev/null \"\$file\" | $_forgit_diff_pager | sed '2 s/added:/untracked:/'
        else
            git diff --color=always -- \"\$file\" | $_forgit_diff_pager
        fi
EOF

I'm also not completely sure why heredoc works fine with escaped quotes and a regular string doesn't, but I thought I'd leave this here as a reference. I can create a PR with this in case we don't find a better solution.

[carlfriedrich]

@sandr01d It works for me with an additional quoted backslash, just like we do it in the preceeding sed call in extract=...:

    preview="
        file=\$(echo {} | $extract)
        if (git status -s -- \\\"\$file\\\" | grep '^??') &>/dev/null; then  # diff with /dev/null for untracked files
            git diff --color=always --no-index -- /dev/null \\\"\$file\\\" | $_forgit_diff_pager | sed '2 s/added:/untracked:/'
        else
            git diff --color=always -- \\\"\$file\\\" | $_forgit_diff_pager
        fi"

Can you verify?

Definitely not the best bash coding style imo, but this seems to be the best quick fix without any further refactoring.

[sandr01d]

@sandr01d It works for me with an additional quoted backslash, just like we do it in the preceeding sed call in extract=...:

    preview="
        file=\$(echo {} | $extract)
        if (git status -s -- \\\"\$file\\\" | grep '^??') &>/dev/null; then  # diff with /dev/null for untracked files
            git diff --color=always --no-index -- /dev/null \\\"\$file\\\" | $_forgit_diff_pager | sed '2 s/added:/untracked:/'
        else
            git diff --color=always -- \\\"\$file\\\" | $_forgit_diff_pager
        fi"

Can you verify?

Definitely not the best bash coding style imo, but this seems to be the best quick fix without any further refactoring.

That makes sense! Can confirm this works, thanks. I'll send a PR your way.