On Tue, May 31, 2011 at 01:32:44AM +0200, Raphaël wrote:
> Hi,
>
> I finished 6 patches which add color to filename completions in
> readline.
Those who tried the patch may have noticed (at least) two bugs:
* non-filename completion (variable, processes, ...) was broken.
I missed part of a condition in readline.c / print_filename().
It is fixed with the attached "dont-bug-non-filename-comp.patch"
* completion of command names was annoying: every match was considered as
a missing-file
It is (partly) fixed with the attached "dont-bug-command-name-completion.patch"
I'll speak about this patch below.
The fact is that readline is not aware that bash went through $PATH and,
very sadly, the char *full_pathname parameter of print_filename() is not
set. readline is, at most, able to check if <full_pathname> or
<to_print> exists (eg, is_dir()).
I was not able to find a nice way to do this without changing a
lot of char **function_definition() by a struct match_entry**.
(it would have had consequences from gen_completion_matches() to
display_matches()).
So I took my inspiration from "rl_filename_completion_desired" and
declared an new readline variable: "rl_executable_completion_desired".
It is almost always 0 but, when
- the bash "command_word_completion_function()" callback is used
- and it returns one entry which is a binary from $PATH
Then
- the variable is set to 1
- the absolute path is returned (instead of the basename)
The lowest common denominator calculation is skipped.
printable_part(), in display_matches(), will take care of splitting the
path so print_filename() will be fed with a valid absolute path (which
allow a correct color).
In such case the LCD would not be useful because:
- there may be aliases, function names, ... + absolute paths
- the LCD on absolute paths would be a non-sense here.
three main drawbacks stays after this patch:
- duplication is not correct:
$ which ls
/bin/ls
$ type -t ls
alias
$ ls<TAB><TAB>
ls ls lsattr ...
[ one is a green (executable), the other is a red "missing file" ]
- aliases, functions, ... and any too "abstract" command name is still
considered as an "missing file". (The goal of the patch was just to make
the "binary in $PATH"-case not that "abstract").
- in the end (print_filename()) priority is given to the filesystem so:
$ pid<TAB><TAB>
pidof@ pidgin*
$ pidt() { true; }
$ pid<TAB><TAB>
pidof@ pidgin* pidt # "pidt" is red as a missing file
$ touch pidt
$ pid<TAB><TAB>
pidof@ pidgin* pidt # "pidt" is blank, that's what stat() decided
I would be happy to be given some advises.
Raph
commit f5136e1f39a7cfbbe3e4177a5d0ee34d2dec987a
Author: Raphaël Droz <[email protected]>
Date: Thu Jun 23 19:34:46 2011 +0200
Colored completion, bugfix (7/6).
Do not skip fnprint() if we are not going to complete filenames.
diff --git a/lib/readline/complete.c b/lib/readline/complete.c
index 1f6ca81..661a5fa 100644
--- a/lib/readline/complete.c
+++ b/lib/readline/complete.c
@@ -835,9 +835,12 @@ print_filename (to_print, full_pathname, prefix_bytes)
extension_char = 0;
/* Postpones printing the filename if we need
- to prefix the output with a color indicator. */
+ to prefix the output with a color indicator.
+ If we complete eg, a $variable, we start output
+ now though.
+ this code may need some reorganization. */
#if defined (COLORED_STATS)
- if (! rl_colored_stats)
+ if (! rl_colored_stats || ! rl_filename_completion_desired)
#endif
printed_len = fnprint (to_print, prefix_bytes);
commit 9c4c7682443c31bab35ea6d1942fb242397d9f4e
Author: Raphaël Droz <[email protected]>
Date: Sat Jun 25 22:55:11 2011 +0200
Colored completion, bugfix (8/6).
When a filename completion needs $PATH traversal (like the
completion of a command name), don't hide the absolute path
to readline.
Matches won't be considered as orphan links anymore when
trying to complete a command name.
diff --git a/bashline.c b/bashline.c
index 692b912..a88fb14 100644
--- a/bashline.c
+++ b/bashline.c
@@ -1453,6 +1453,10 @@ bash_default_completion (text, start, end, qc, compflags)
matches = (char **)NULL;
+ /* the "command_word_completion_function" callback may change this value
+ in case we use colored filenames completion. */
+ rl_executable_completion_desired = 0;
+
/* New posix-style command substitution or variable name? */
if (!matches && *text == '$')
{
@@ -1899,12 +1903,20 @@ globword:
bash execution code won't find executables in directories which
appear in directories in $PATH when they're specified using
relative pathnames. */
- if (match && (searching_path ? executable_file (val) :
executable_or_directory (val)))
+ int is_executable_file = 0;
+ if (match && (searching_path ? ( is_executable_file = executable_file
(val) ) : executable_or_directory (val)))
#endif
{
- free (val);
- val = ""; /* So it won't be NULL. */
- return (temp);
+ if(is_executable_file) {
+ rl_executable_completion_desired = 1;
+ free (temp);
+ return val;
+ }
+ else {
+ free (val);
+ val = ""; /* So it won't be NULL. */
+ return (temp);
+ }
}
else
{
diff --git a/lib/readline/complete.c b/lib/readline/complete.c
index 661a5fa..f921d56 100644
--- a/lib/readline/complete.c
+++ b/lib/readline/complete.c
@@ -317,6 +317,15 @@ int rl_ignore_completion_duplicates = 1;
within a completion entry finder function. */
int rl_filename_completion_desired = 0;
+/* Non-zero means that at least one of the results of the matches is
+ an absolute path. This is ALWAYS zero on entry.
+ If it is set to a non-zero value the lower common denominator will
+ not be computed.
+ The matches should expect to be processed by printable_part().
+ print_filename() will receive a correct full_pathname parameter.
+*/
+int rl_executable_completion_desired = 0;
+
/* Non-zero means that the results of the matches are to be quoted using
double quotes (or an application-specific quoting mechanism) if the
filename contains any characters in rl_filename_quote_chars. This is
@@ -2077,8 +2086,21 @@ rl_completion_matches (text, entry_function)
/* If there were any matches, then look through them finding out the
lowest common denominator. That then becomes match_list[0]. */
- if (matches)
- compute_lcd_of_matches (match_list, matches, text);
+ if (matches) {
+ /* ... except if we are doing filename completion and entry_function()
+ returned one or more absolute path(s).
+ The lowest common denominator would not be useful so we
+ keep the hint_text as-is.
+ printable_part() will strip the directory name later.
+ Duplicates will not be removed.
+ */
+ if( rl_executable_completion_desired == 1) {
+ match_list[0] = (char *)xmalloc (strlen (text) + 1);
+ strcpy (match_list[0], text);
+ }
+ else
+ compute_lcd_of_matches (match_list, matches, text);
+ }
else /* There were no matches. */
{
xfree (match_list);
diff --git a/lib/readline/readline.h b/lib/readline/readline.h
index dc8b29a..b7ba6c8 100644
--- a/lib/readline/readline.h
+++ b/lib/readline/readline.h
@@ -747,6 +747,15 @@ extern rl_compdisp_func_t
*rl_completion_display_matches_hook;
within a completion entry finder function. */
extern int rl_filename_completion_desired;
+/* Non-zero means that at least one of the results of the matches is
+ an absolute path. This is ALWAYS zero on entry.
+ If it is set to a non-zero value the lower common denominator will
+ not be computed.
+ The matches should expect to be processed by printable_part().
+ print_filename() will receive a correct full_pathname parameter.
+*/
+extern int rl_executable_completion_desired;
+
/* Non-zero means that the results of the matches are to be quoted using
double quotes (or an application-specific quoting mechanism) if the
filename contains any characters in rl_word_break_chars. This is
