bug-readline
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

rl_filename_rewrite_hook called on wrong filename


From: Stefan H. Holek
Subject: rl_filename_rewrite_hook called on wrong filename
Date: Sun, 24 Sep 2023 12:38:22 +0200

Hi All,

There appears to be an issue with a recent addition to 
rl_filename_completion_function. It now applies rl_filename_rewrite_hook to the 
filename part of "what the user has typed". This seems wrong. Let me explain.

The rl_filename_rewrite_hook exists to convert data read from the filesystem to 
a representation that works in the terminal. E.g. on macOS the filesystem 
returns decomposed UTF-8, which must be converted to fully composed UTF-8 
before comparing it to a string the user has typed.

Now, the section below (in complete.c) appears to apply 
rl_filename_rewrite_hook to a string in TERMINAL representation ('filename' is 
the rightmost part of the path the user has typed):


@@ -2542,6 +2536,18 @@ rl_filename_completion_function (const char *text, int 
state)
        }
      filename_len = strlen (filename);

+      /* Normalize the filename if the application has set a rewrite hook. */
+      if (*filename && rl_filename_rewrite_hook)
+       {
+         temp = (*rl_filename_rewrite_hook) (filename, filename_len);
+         if (temp != filename)
+           {
+             xfree (filename);
+             filename = temp;
+             filename_len = strlen (filename);
+           }
+       }
+
      rl_filename_completion_desired = 1;
    }


I struggle to find this useful and in fact think it's dangerous and should be 
backed out.

If I have an rl_filename_rewrite_hook that works in Readline 8.2, it may just 
fail in 8.3 because it is applied to a string that is not in the expected 
filesystem representation!

Readline has so far worked fine in scenarios where the terminal encoding 
differs from the filesystem encoding. I can use rl_directory_rewrite_hook and 
rl_filename_stat_hook to go from terminal -> filesystem encoding, and 
rl_filename_rewrite_hook to go from filesystem -> terminal encoding. It is my 
understanding that these hooks have been added to support this use-case in the 
first place.

Adding the above snippet breaks this contract. Please reconsider.

Thank you,
Stefan

-- 
Stefan H. Holek
stefan@epy.co.at




reply via email to

[Prev in Thread] Current Thread [Next in Thread]