Fix escaping for completion on cmdline (spaces, backslashes, etc).

[Patrick Winnertz <winnie@…>]

Hey,

This patch will fix problems with escaping on cmdline basically. The patch is
not perfect and should, at least, partially reworked.

Greetings
Winnie

[Patrick Winnertz]

Added by email2trac

[Patrick Winnertz]

This message has 1 attachment(s)

[Patrick Winnertz]

I'll work on this..

please review the patch which is attached to this ticket

[Patrick Winnertz]

(In #149) Sadly it's not possible to set the stuff without writing a bit or in the first
mail:

I'll work on this and this should be fixe in 4.6.2.

[winnie]

Maybe it's a good idea to try to use

g_strescape and g_strcompress instead of home-made functions...

[Patrick Winnertz]

Hey,

Maybe it's a good idea to try to use

g_strescape and g_strcompress instead of home-made functions...

I tried it with these functions.. unfortunatly g_strescape doesn't escape the
whitespace .. therefore this function is useless for us.

I rewrote the patch a little bit to escape everything even if it's the leading
char.

[Patrick Winnertz]

Added by email2trac

[Patrick Winnertz]

Added by email2trac

[Patrick Winnertz]

Hey Roland,

See the branch this commit belongs too, and you'll see that this belongs to a
patch where some rework is needed. :) See ticket:147.

In order to get your remark not only onto this list but also into the ticket
'm replying not the the list but to the ticketsystem which will then forward
the mail back to the list.

I don't get why there has to be a variable k in the functions. Its
completely unnecessary. A much simpler, faster, and more reliable way is
the following (untested):

I'll test it and merge it into the bugfix branch.

char *
unescape_string(const char *in) {

GString *str;
const char * src;
char *result;

str = g_string_new("");

for (src = in; *src != '\0'; src++) {

if (src[0] == '
' && strchr(" \t*|;<>~#()?[]{}&", src[1])) {

g_string_append_c(str, src[1]);
src++;

} else {

g_string_append_c(str, src[0]);

}

}

result = str->str;
g_string_free(str, FALSE);
return result;

}

It's much simpler that way. It doesn't call strlen() unnecessarily. It
doesn't have unneeded variables. And it doesn't crash if in[-1] is an
illegal address.

Well It is indeed much simpler.. but do you tested this patch once? mc won't
crash...(tested).

Greetings
Winnie

[rillig]

In the unescape_string function, there is no need to check for (i == 0). Otherwise the first character of the string may be skipped, which I think is not intended.

Why do the following characters do not need escaping:?

$ `

and why aren't the characters sorted according to their ASCII values?

Roland

[Patrick Winnertz]

Hey,

Why do the following characters do not need escaping:?

$ `

Added.

and why aren't the characters sorted according to their ASCII values?

This is done now.

Please note that I had not much time to rewrite the escape_string function in
order to use g_str_append_c. It uses atm a negative index for a check
(src[-1]). It looks ugly but it doesn't fail. Please send a patch if you have
a better idea for this ifclause.

Greetings
Winnie

[winnie]

(In #10) There is a fix in the 10_fish_stalls_on_symlink branch.. however this fix is uncomplete as it will fail in this scenario:

echo "ALL OK" >"this -> file"
ln -s "this -> file" "this -> link"

The link will point to -> file instead of "this -> file".

To fix this issue we need quoted output. (This means ls -Q). As this is no standard option but only used in gnu ls we have to check prior using it if this option is available.
However this is related to ticket:149.

[winnie]

Use now g_str_append_c and use ascii values to sort symbols

[slavazanko]

1. run mc with patch
2. type cd ~ and press twice ESC,TAB
3. select from list your homedir (\~slavaz/ in my case)
4. press twice ESC,TAB

no any reaction (only bells).

Without patch continued to navigate:

5. select from list subdir in your homedir
6. press twice ESC,TAB
7. ...

After remove line src/complete.c:947 (see escaping-patch-rev3.patch) navigate to subdirs worked.

[slavazanko]

BTW:

diff --git a/src/complete.c b/src/complete.c
index a11b9c9..cb18f42 100644
--- a/src/complete.c
+++ b/src/complete.c
@@ -944,7 +944,9 @@ complete_engine (WInput *in, int what_to_do)
            WListbox *query_list;

            for (p=in->completions + 1; *p; count++, p++) {
-           *p = escape_string(*p);
+               q = escape_string(*p);
+               g_free(*p);
+               *p = q;
                if ((i = strlen (*p)) > maxlen)
                    maxlen = i;
                        }

I think, this remove some memory leaks...

And need to review some other parts in patch. For example src/file.c:1876

source_with_path = unescape_string(source_with_path); 

I think, in any case need to free memory by old pointer. Example:

char *old_pnt;

...
old_pnt = source_with_path;
source_with_path = unescape_string(source_with_path); 
g_free(old_pnt);

and others like this...

[slavazanko]

fix some memory leaks. WARNING: patch don't allyped to '147_escaping' branch

[slavazanko]

fix for attachment comment:

- allyped
+ applyed

:)

Next testing stuff:

1. press F7 (make dir)
2. type: this dir
3. press SHIFT+F6 (rename)
4. change 'this dir' to 'this \ dir'

Got error.

5. press SHIFT+F6 (rename) again
6. change 'this dir' to 'this
   dir'

Got error.

[slavazanko]

duplicate #152

[winnie]

This needs a rework by me after #157 is merged.

[slavazanko]

Well... Now in '147_escaping' fixed completion bug, but behaviour present with renaming 'this dir' to 'this \ dir'.

Please, test this branch and say your thinks here.

[slavazanko]

Well.. Bug with renaming/copying fixed. See git-log and git-diff in branch '147_escaping'.

Please, review and vote.

[winnie]

This is still based on the functions escape_string & unescape_string.. This is now integrated into mhl, please use this stuff.

[winnie]

(In #10) removed #147 as blocking bug. the escape functions are now provided inside the mhl lib which was addressed in #157

[winnie]

remove #149 as blocked bug as #149 needs only the escaping functions which are now provides by the mhl lib which was added in #157

[slyfox]

yeah, mhl stuff would reduce this patch a _lot_

[slavazanko]

See 147_escape_use_MHL.patch - just apply to HEAD of 147_... branch
review, please.

[slavazanko]

patch 147_escape_use_MHL.patch applied to '147' branch.

Review, test and vote, please.

[winnie]

I've still some troubles with this patch.. e.g have a look on this testscenario:

• create /tmp/test
• in /tmp/test/ (touch them):

  -rw-r--r-- 1 winnie winnie 0 27. Jan 20:35 " file"
  -rw-r--r-- 1 winnie winnie 0 27. Jan 20:35 " file$test"
  -rw-r--r-- 1 winnie winnie 0 27. Jan 20:35 " file&test"
  

Now type on subshell:

less /tmp/test/

and press twice ESC-tab. Normally a completion dialog should open but this doesn't happen here. (You can verify this with a simple test):

less /tmp/

and now press twice ESC-tab.

So.. this needs to be fixed.

[metux]

I've created some dir "/tmp/ foo" and opened the copy dialog.
Typing in /tmp and pressing esc-tab twice shows up " foo" properly, and selecting it works fine.

BUT: typing in "/tmp/ " and pressing esc-tab twice shows my homedir in the list.

[metux]

Found out that the filename_completion_function doesnt get the leading spaces passed
(create some dir "/tmp/ foo" and try to expand "/tmp ")

[slyfox]

Partially reworked completion engine: adedd INPUT_COMPLETION_FLAGS state to all completion functions, which helps to separate completion contexts.

147_escaping fixes:

• completion of regular files after command:

  -rw-r--r-- 1 winnie winnie 0 27. Jan 20:35 " file"
  -rw-r--r-- 1 winnie winnie 0 27. Jan 20:35 " file$test"
  -rw-r--r-- 1 winnie winnie 0 27. Jan 20:35 " file&test"
  $ less /tmp/<complete>
  $ less ./<complete>
  

• cd command now acts as \cd (accepts escaped dirs)

[angel_il]

it works

[winnie]

Okay.. branch works very fine :) Vote from me

[winnie]

Okay finally closing this bug :) This was the last showstopper for 4.6.2