[v2,2/5] gdb: Tab complete internalvars in expressions

Message ID 20240828014916.162446-3-nt8r@protonmail.com
State New
Headers
Series Tab complete convenience variables |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Antonio Rische Aug. 28, 2024, 1:50 a.m. UTC
  For example, 'print $f<tab>' or 'print $fo<tab>+$foo' after running
'set $foo=0' now tab completes.

'print $_siginf<tab>' also now tab completes.
---
 gdb/completer.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
  

Comments

Keith Seitz Aug. 30, 2024, 7:32 p.m. UTC | #1
On 8/27/24 6:50 PM, Antonio Rische wrote:
> For example, 'print $f<tab>' or 'print $fo<tab>+$foo' after running
> 'set $foo=0' now tab completes.
> 
> 'print $_siginf<tab>' also now tab completes.
> ---
>   gdb/completer.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/gdb/completer.c b/gdb/completer.c
> index 1008ec23b..3c9f01c84 100644
> --- a/gdb/completer.c
> +++ b/gdb/completer.c
> @@ -1099,6 +1099,17 @@ complete_expression (completion_tracker &tracker,
>         && expr_completer->complete (exp.get (), tracker))
>       return;
>   
> +  /* If the text is non-empty, see if the word is preceded by '$'.  */
> +  if (text[0] != '\0' && text[strlen(text)-strlen(word)-1] == '$')

This should be formatted: text[strlen (text) - strlen (word) - 1]

> +	{
> +	  tracker.advance_custom_word_point_by (1);
> +	  /* We don't support completion of history indices.  */
> +	  if (!isdigit (word[0]))
> +	    complete_internalvar (tracker, word);
> +	  tracker.advance_custom_word_point_by (-1);
> +	  return;
> +	}
> +
>     complete_files_symbols (tracker, text, word);
>   }
>   

I don't follow the use of advance_custom_word_point here, advancing
by 1 and then reversing it. I don't seen that used anywhere else,
either. Can you explain?

The behavior here is also a little strange with double TAB:

(gdb) p $_gdbTABTAB
_gdb_maint_setting      _gdb_major              _gdb_setting
_gdb_maint_setting_str  _gdb_minor              _gdb_setting_str

I would expect this to include the leading '$'.

Keith
  
Hannes Domani Aug. 30, 2024, 7:54 p.m. UTC | #2
Am Mittwoch, 28. August 2024 um 03:50:58 MESZ hat Antonio Rische <nt8r@protonmail.com> Folgendes geschrieben:

> For example, 'print $f<tab>' or 'print $fo<tab>+$foo' after running
> 'set $foo=0' now tab completes.
>
> 'print $_siginf<tab>' also now tab completes.
> ---
> gdb/completer.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/gdb/completer.c b/gdb/completer.c
> index 1008ec23b..3c9f01c84 100644
> --- a/gdb/completer.c
> +++ b/gdb/completer.c
> @@ -1099,6 +1099,17 @@ complete_expression (completion_tracker &tracker,
>       && expr_completer->complete (exp.get (), tracker))
>     return;
>
> +  /* If the text is non-empty, see if the word is preceded by '$'.  */
> +  if (text[0] != '\0' && text[strlen(text)-strlen(word)-1] == '$')
> +    {
> +      tracker.advance_custom_word_point_by (1);
> +      /* We don't support completion of history indices.  */
> +      if (!isdigit (word[0]))
> +        complete_internalvar (tracker, word);
> +      tracker.advance_custom_word_point_by (-1);
> +      return;
> +    }
> +
>   complete_files_symbols (tracker, text, word);
>
> }
>
> --
> 2.46.0

If this were done as the first thing in the complete_expression function,
then patch 1 wouldn't be necessary.
But I guess there is a good reason this is done after the expr_completer
stuff, like '$' being handled by some language.

Also, this series fixes PR7353, so you can add this in the commit messages
of patch 2 and 4:
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7353


Hannes
  
Antonio Rische Sept. 1, 2024, 9:35 p.m. UTC | #3
The `advance_custom_word_point` usage was a holdover from when I was confused about the contract for the completer code; it isn't necessary and I'll remove it in v3 along with doing all formatting fixes.

The completion code could have suggested completions include the $, but that isn't what's done by `complete_internalvar` or `complete_register` internally. Effectively we just treat the $ as context. Is it straightforward to also include the $ in the completions? If not, I'd say it probably isn't worth the trouble.

Antonio

On Friday, August 30th, 2024 at 7:32 PM, Keith Seitz <keiths@redhat.com> wrote:

> On 8/27/24 6:50 PM, Antonio Rische wrote:
> 
> > For example, 'print $f<tab>' or 'print $fo<tab>+$foo' after running
> > 'set $foo=0' now tab completes.
> > 
> > 'print $_siginf<tab>' also now tab completes.
> > ---
> > gdb/completer.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> > 
> > diff --git a/gdb/completer.c b/gdb/completer.c
> > index 1008ec23b..3c9f01c84 100644
> > --- a/gdb/completer.c
> > +++ b/gdb/completer.c
> > @@ -1099,6 +1099,17 @@ complete_expression (completion_tracker &tracker,
> > && expr_completer->complete (exp.get (), tracker))
> > return;
> > 
> > + /* If the text is non-empty, see if the word is preceded by '$'. */
> > + if (text[0] != '\0' && text[strlen(text)-strlen(word)-1] == '$')
> 
> 
> This should be formatted: text[strlen (text) - strlen (word) - 1]
> 
> > + {
> > + tracker.advance_custom_word_point_by (1);
> > + /* We don't support completion of history indices. */
> > + if (!isdigit (word[0]))
> > + complete_internalvar (tracker, word);
> > + tracker.advance_custom_word_point_by (-1);
> > + return;
> > + }
> > +
> > complete_files_symbols (tracker, text, word);
> > }
> 
> 
> I don't follow the use of advance_custom_word_point here, advancing
> by 1 and then reversing it. I don't seen that used anywhere else,
> either. Can you explain?
> 
> The behavior here is also a little strange with double TAB:
> 
> (gdb) p $_gdbTABTAB
> _gdb_maint_setting _gdb_major _gdb_setting
> _gdb_maint_setting_str _gdb_minor _gdb_setting_str
> 
> I would expect this to include the leading '$'.
> 
> Keith
  

Patch

diff --git a/gdb/completer.c b/gdb/completer.c
index 1008ec23b..3c9f01c84 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1099,6 +1099,17 @@  complete_expression (completion_tracker &tracker,
       && expr_completer->complete (exp.get (), tracker))
     return;
 
+  /* If the text is non-empty, see if the word is preceded by '$'.  */
+  if (text[0] != '\0' && text[strlen(text)-strlen(word)-1] == '$')
+	{
+	  tracker.advance_custom_word_point_by (1);
+	  /* We don't support completion of history indices.  */
+	  if (!isdigit (word[0]))
+	    complete_internalvar (tracker, word);
+	  tracker.advance_custom_word_point_by (-1);
+	  return;
+	}
+
   complete_files_symbols (tracker, text, word);
 }