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 <raphael.droz+fl...@gmail.com> 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 <raphael.droz+fl...@gmail.com> 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