Wrong argument quoting when filename uses spaces.

[grota]

hi @ndbroadbent, I seem to be getting some quoting errors when using the aliased mv using filenames with spaces, here's an example (with bash)

> mv Tungevaag\ \&\ Raaban\,\ Emila\ -Samsara.mp3 Tungevaag\ \&\ Raaban\,\ Emila\ -\ Samsara.mp3                  
[1] 30313
[2] 30314
/bin/mv: missing destination file operand after ‘Tungevaag ’
Try '/bin/mv --help' for more information.
 Raaban, Emila -Samsara.mp3: command not found
 Raaban, Emila - Samsara.mp3: command not found
[1]-  Exit 1                  /bin/mv Tungevaag\ 
[2]+  Exit 127                \ Raaban,\ Emila\ -Samsara.mp3 Tungevaag\ 

> type mv
mv is aliased to `exec_scmb_expand_args /bin/mv'

> type exec_scmb_expand_args
exec_scmb_expand_args is a function
exec_scmb_expand_args () 
{ 
    eval "$(scmb_expand_args "$@" | sed -e "s/\([][|;()<>^ \"']\)/"'\\\1/g')"
}

I think the right way to deal with would be to use something along the lines of printf "%q" "$variable". I can work on that, would you be accepting a MR using this strategy?

It is also possible that such a patch would fix another couple of issues that have already been reported.

[sudocurse]

bump. Experiencing this too (probably because my mv is setup as an alias to mv -i?)

[ghthor]

@sudocurse @grota I'm looking at this now.

I think perhaps we need to dig deeper, the bug might be in this nasty looking function.

> type scmb_expand_args
scmb_expand_args is a function
scmb_expand_args () 
{ 
    if [ "$1" = "--relative" ]; then
        local relative=1;
        shift;
    fi;
    first=1;
    OLDIFS="$IFS";
    IFS=" ";
    for arg in "$@";
    do
        if [[ "$arg" =~ ^[0-9]{0,4}$ ]]; then
            if [ "$first" -eq 1 ]; then
                first=0;
            else
                printf '\t';
            fi;
            if [ -e "$arg" ]; then
                printf '%s' "$arg";
            else
                _print_path "$relative" "$git_env_char$arg";
            fi;
        else
            if [[ "$arg" =~ ^[0-9]+-[0-9]+$ ]]; then
                for i in $(eval echo {${arg/-/..}});
                do
                    if [ "$first" -eq 1 ]; then
                        first=0;
                    else
                        printf '\t';
                    fi;
                    _print_path "$relative" "$git_env_char$i";
                done;
            else
                if [ "$first" -eq 1 ]; then
                    first=0;
                else
                    printf '\t';
                fi;
                printf '%s' "$arg";
            fi;
        fi;
    done;
    IFS="$OLDIFS"
}

Also, I'm not entirely sure what this piped sed -e "s/\([][|;()<>^ \"']\)/"'\\\1/g' command is doing exactly? I wouldn't mind if we could determine what it's doing to extract it and name it as a func to help document the intention here.

[ghthor]

@HaleTom Will your work on #255 help with this bug at all?

[HaleTom]

I'm not working on #255 yet. However:

• I have reworked scmb_expand_args to return an array.
• I've written a wrapper around eval doing the equivalent of the printf '%q as suggested above.
• My current code is linked from #204. I just tested and mv 'two words' 'a b' worked.

That sed looked idempotent to me, and is moot given the wrapper.

Could someone write failing tests for this particular issue?

[HaleTom]

Ah, this is probably the & character backgrounding a task.

My code:

% alias mv
mv='exec_scmb_expand_args /usr/bin/mv -i'
% touch 'a & b'
% mv 'a & b' 'b & c'
% ls 'b & c'
'b & c'
%

I have a bug preventing passing tests and will soon make a PR for review.

[ghthor]

For anyone interested in what the hell that sed command was doing, thanks to wizard @schuess.

The goal seems to be to preserve whatever delimiters are passed in via $@ in the expanded command.

Here’s how I break it down, I hope this is useful:

Start with sed -e "s/\([][|;()<>^ \"']\)/"'\\\1/g')

Remove trailing ) because that’s part of the eval that this statement is embedded in:

sed -e "s/\([][|;()<>^ \"']\)/"'\\\1/g'

Remove internal quotes, since that appears to be controlling evaluation (double-quotes for the find mean it gets evaluated right away and single-quotes for the replace mean that is evaluated later):

sed -e s/\([][|;()<>^ \"']\)/\\\1/g

Remove -e since you aren’t stringing multiple sed commands:

sed s/\([][|;()<>^ \"']\)/\\\1/g

Change the substitute delimiter from / to # to make the divisions easier to see:

sed s#\([][|;()<>^ \"']\)#\\\1#g

Remove the double backslash \\ inside the “don’t evaluate yet” single quotes so this can run correctly on my mac command line:

sed s#\([][|;()<>^ \"']\)#\1#g

Quote the final substitute command:

sed "s#\([][|;()<>^ \"']\)#\1#g"

Create a test file:

echo "a]b[c|d;e(f)g<h>i^j k\"l'm" > test.txt

Run the command:

cat test.txt; sed "s#\([][|;()<>^ \"']\)#\1#g" test.txt

You can add an arbitrary symbol after the capture reference to see what gets captured and reinserted:

cat test.txt; sed "s#\([][|;()<>^ \"']\)#\1!#g" test.txt

[HaleTom]

Please check #263 and confirm if this is resolved.

[grota]

I gave a cursory look at the diff of that PR and it looks good. Sorry I can't actually test if that code solves the original issue

[ghthor]

@grota Have you updated .scm_breeze since we merged #263 to report back if this is indeed fixed?

[grota]

I pulled from upstream and I have rm is aliased to 'exec_scmb_expand_args /bin/rm' and yeah, it's all good