Add shortcuts for "nexti" and "stepi" commands in Single-Key mode

  Currently, "layout asm" is not so useful as "layout src" with Single-Key mode:
you have to use multi-key commands like "ni" and "si" to do single-stepping.
This patch adds, in addition to "next" and "step" commands, corresponding
assembly-level ones - "nexti" and "stepi" - to Single-Key mode, with the
shortcuts of "N" and "S" respectively.

gdb/ChangeLog:

	* tui/tui.c: Add "nexti" and "stepi" to the tui_commands list
	* doc/gdb.texinfo: Document the new shortcuts in Single-Key mode
---
 gdb/doc/gdb.texinfo |    8 ++++++++
 gdb/tui/tui.c       |    2 ++
 2 files changed, 10 insertions(+)
  

> From: Ruslan Kabatsayev <b7.10110111@gmail.com>
> Cc: Ruslan Kabatsayev <b7.10110111@gmail.com>
> Date: Thu, 13 Jul 2017 09:07:50 +0300
> 
> Currently, "layout asm" is not so useful as "layout src" with Single-Key mode:
> you have to use multi-key commands like "ni" and "si" to do single-stepping.
> This patch adds, in addition to "next" and "step" commands, corresponding
> assembly-level ones - "nexti" and "stepi" - to Single-Key mode, with the
> shortcuts of "N" and "S" respectively.
> 
> gdb/ChangeLog:
> 
> 	* tui/tui.c: Add "nexti" and "stepi" to the tui_commands list
> 	* doc/gdb.texinfo: Document the new shortcuts in Single-Key mode

This is fine with me, but please state in the ChangeLog the
functions/globals where you make the changes, and for gdb.texinfo
please state the name of the node (in parentheses).  I think the
existing ChangeLog entries show how we do that.

Thanks.
  

On 13/07/17 18:59, Eli Zaretskii wrote:
>> From: Ruslan Kabatsayev <b7.10110111@gmail.com>
>> Cc: Ruslan Kabatsayev <b7.10110111@gmail.com>
>> Date: Thu, 13 Jul 2017 09:07:50 +0300
>>
>> Currently, "layout asm" is not so useful as "layout src" with Single-Key mode:
>> you have to use multi-key commands like "ni" and "si" to do single-stepping.
>> This patch adds, in addition to "next" and "step" commands, corresponding
>> assembly-level ones - "nexti" and "stepi" - to Single-Key mode, with the
>> shortcuts of "N" and "S" respectively.
>>
>> gdb/ChangeLog:
>>
>> 	* tui/tui.c: Add "nexti" and "stepi" to the tui_commands list
>> 	* doc/gdb.texinfo: Document the new shortcuts in Single-Key mode
> 
> This is fine with me, but please state in the ChangeLog the
> functions/globals where you make the changes, and for gdb.texinfo
> please state the name of the node (in parentheses).  I think the
> existing ChangeLog entries show how we do that.

gdb/ChangeLog:
	* tui/tui.c (tui_commands): Add "nexti" and "stepi" to the tui_commands list
	* doc/gdb.texinfo (TUI Single Key Mode): Document the new shortcuts in
	Single-Key mode

> 
> Thanks.
>
  

> Cc: gdb-patches@sourceware.org
> From: Ruslan Kabatsayev <b7.10110111@gmail.com>
> Date: Thu, 13 Jul 2017 21:19:10 +0300
> 
> >> gdb/ChangeLog:
> >>
> >> 	* tui/tui.c: Add "nexti" and "stepi" to the tui_commands list
> >> 	* doc/gdb.texinfo: Document the new shortcuts in Single-Key mode
> > 
> > This is fine with me, but please state in the ChangeLog the
> > functions/globals where you make the changes, and for gdb.texinfo
> > please state the name of the node (in parentheses).  I think the
> > existing ChangeLog entries show how we do that.
> 
> gdb/ChangeLog:
> 	* tui/tui.c (tui_commands): Add "nexti" and "stepi" to the tui_commands list
> 	* doc/gdb.texinfo (TUI Single Key Mode): Document the new shortcuts in
> 	Single-Key mode

Yes, that's it, except that the lines should be broken at column 75,
and there should be a period at the end of each sentence.

Thanks.
  

On 13/07/17 22:04, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Ruslan Kabatsayev <b7.10110111@gmail.com>
>> Date: Thu, 13 Jul 2017 21:19:10 +0300
>>
>>>> gdb/ChangeLog:
>>>>
>>>> 	* tui/tui.c: Add "nexti" and "stepi" to the tui_commands list
>>>> 	* doc/gdb.texinfo: Document the new shortcuts in Single-Key mode
>>>
>>> This is fine with me, but please state in the ChangeLog the
>>> functions/globals where you make the changes, and for gdb.texinfo
>>> please state the name of the node (in parentheses).  I think the
>>> existing ChangeLog entries show how we do that.
>>
>> gdb/ChangeLog:
>> 	* tui/tui.c (tui_commands): Add "nexti" and "stepi" to the tui_commands list
>> 	* doc/gdb.texinfo (TUI Single Key Mode): Document the new shortcuts in
>> 	Single-Key mode
> 
> Yes, that's it, except that the lines should be broken at column 75,
> and there should be a period at the end of each sentence.

gdb/ChangeLog:
	* tui/tui.c (tui_commands): Add "nexti" and "stepi" to the Single-Key
	mode command list.
	* doc/gdb.texinfo (TUI Single Key Mode): Document the new shortcuts in
	Single-Key mode.

> 
> Thanks.
>
  

> Cc: gdb-patches@sourceware.org
> From: Ruslan Kabatsayev <b7.10110111@gmail.com>
> Date: Thu, 13 Jul 2017 22:08:53 +0300
> 
> gdb/ChangeLog:
> 	* tui/tui.c (tui_commands): Add "nexti" and "stepi" to the Single-Key
> 	mode command list.
> 	* doc/gdb.texinfo (TUI Single Key Mode): Document the new shortcuts in
> 	Single-Key mode.

Perfect, thanks.
  

So, the patch seems to have been approved by one reviewer (see [1]). But it
doesn't appear to have been pushed, nor do I have any further instructions
on how to get it merged. What should I do?

[1]: https://sourceware.org/ml/gdb-patches/2017-07/msg00165.html

On 13/07/17 09:07, Ruslan Kabatsayev wrote:
> Currently, "layout asm" is not so useful as "layout src" with Single-Key mode:
> you have to use multi-key commands like "ni" and "si" to do single-stepping.
> This patch adds, in addition to "next" and "step" commands, corresponding
> assembly-level ones - "nexti" and "stepi" - to Single-Key mode, with the
> shortcuts of "N" and "S" respectively.
> 
> gdb/ChangeLog:
> 
> 	* tui/tui.c: Add "nexti" and "stepi" to the tui_commands list
> 	* doc/gdb.texinfo: Document the new shortcuts in Single-Key mode
> ---
>  gdb/doc/gdb.texinfo |    8 ++++++++
>  gdb/tui/tui.c       |    2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index c167a86..56163ab 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -25415,6 +25415,10 @@ finish
>  @item n
>  next
>  
> +@kindex N @r{(SingleKey TUI key)}
> +@item N
> +nexti
> +
>  @kindex q @r{(SingleKey TUI key)}
>  @item q
>  exit the SingleKey mode.
> @@ -25427,6 +25431,10 @@ run
>  @item s
>  step
>  
> +@kindex S @r{(SingleKey TUI key)}
> +@item S
> +stepi
> +
>  @kindex u @r{(SingleKey TUI key)}
>  @item u
>  up
> diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
> index c918f3e..dbd86e8 100644
> --- a/gdb/tui/tui.c
> +++ b/gdb/tui/tui.c
> @@ -75,8 +75,10 @@ static const struct tui_char_command tui_commands[] = {
>    { 'd', "down" },
>    { 'f', "finish" },
>    { 'n', "next" },
> +  { 'N', "nexti" },
>    { 'r', "run" },
>    { 's', "step" },
> +  { 'S', "stepi" },
>    { 'u', "up" },
>    { 'v', "info locals" },
>    { 'w', "where" },
>
  

On 2017-07-26 21:19, Ruslan Kabatsayev wrote:
> So, the patch seems to have been approved by one reviewer (see [1]). 
> But it
> doesn't appear to have been pushed, nor do I have any further 
> instructions
> on how to get it merged. What should I do?

Hi Ruslan,

The patch is fine with me too, it seems to work well and it's a good 
idea.

It might be a bit more work to implement (an idea for another patch 
maybe), but do you think it would be nice if n and s did next and step 
when focusing a source window, but did nexti and stepi when focusing an 
asm window?  When in an asm window, I think you will much more likely 
want to use nexti/stepi than next/step, so maybe they should be bound to 
the n and s rather than N and S in that case?

In order to get the patch in, you have two choices: if you plan to 
contribute more in the future, we can get you write access to the repo, 
so that you can push your patches in once they are approved.  Otherwise, 
we can push it for you.  Let me know what you prefer.

Thanks,

Simon
  

On 26 July 2017 at 23:53, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> On 2017-07-26 21:19, Ruslan Kabatsayev wrote:
>>
>> So, the patch seems to have been approved by one reviewer (see [1]). But
>> it
>> doesn't appear to have been pushed, nor do I have any further instructions
>> on how to get it merged. What should I do?
>
>
> Hi Ruslan,
>
> The patch is fine with me too, it seems to work well and it's a good idea.
>
> It might be a bit more work to implement (an idea for another patch maybe),
> but do you think it would be nice if n and s did next and step when focusing
> a source window, but did nexti and stepi when focusing an asm window?  When
> in an asm window, I think you will much more likely want to use nexti/stepi
> than next/step, so maybe they should be bound to the n and s rather than N
> and S in that case?
This sounds like a great idea. If this is implemented, do you think N
and S are then useless and confusing (since n/s would be
focus-dependent while N/S won't)?

>
> In order to get the patch in, you have two choices: if you plan to
> contribute more in the future, we can get you write access to the repo, so
> that you can push your patches in once they are approved.  Otherwise, we can
> push it for you.  Let me know what you prefer.
Yes, I do plan to contribute in the future.

>
> Thanks,
>
> Simon
>
  

On 2017-07-27 07:59, Ruslan Kabatsayev wrote:
> On 26 July 2017 at 23:53, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>> On 2017-07-26 21:19, Ruslan Kabatsayev wrote:
>>> 
>>> So, the patch seems to have been approved by one reviewer (see [1]). 
>>> But
>>> it
>>> doesn't appear to have been pushed, nor do I have any further 
>>> instructions
>>> on how to get it merged. What should I do?
>> 
>> 
>> Hi Ruslan,
>> 
>> The patch is fine with me too, it seems to work well and it's a good 
>> idea.
>> 
>> It might be a bit more work to implement (an idea for another patch 
>> maybe),
>> but do you think it would be nice if n and s did next and step when 
>> focusing
>> a source window, but did nexti and stepi when focusing an asm window?  
>> When
>> in an asm window, I think you will much more likely want to use 
>> nexti/stepi
>> than next/step, so maybe they should be bound to the n and s rather 
>> than N
>> and S in that case?
> This sounds like a great idea. If this is implemented, do you think N
> and S are then useless and confusing (since n/s would be
> focus-dependent while N/S won't)?

I was thinking that perhaps that when focusing...

a source view: s/n do source line stepping, S/N do assembly stepping
an asm view: s/n do assembly stepping, S/N do source line stepping

Basically, they would switch role.  Is that too confusing?

>> 
>> In order to get the patch in, you have two choices: if you plan to
>> contribute more in the future, we can get you write access to the 
>> repo, so
>> that you can push your patches in once they are approved.  Otherwise, 
>> we can
>> push it for you.  Let me know what you prefer.
> Yes, I do plan to contribute in the future.

I'll send you the details in private.

Thanks,

Simon
  

On 27 July 2017 at 11:19, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> On 2017-07-27 07:59, Ruslan Kabatsayev wrote:
>>
>> On 26 July 2017 at 23:53, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>>
>>> On 2017-07-26 21:19, Ruslan Kabatsayev wrote:
>>>>
>>>>
>>>> So, the patch seems to have been approved by one reviewer (see [1]). But
>>>> it
>>>> doesn't appear to have been pushed, nor do I have any further
>>>> instructions
>>>> on how to get it merged. What should I do?
>>>
>>>
>>>
>>> Hi Ruslan,
>>>
>>> The patch is fine with me too, it seems to work well and it's a good
>>> idea.
>>>
>>> It might be a bit more work to implement (an idea for another patch
>>> maybe),
>>> but do you think it would be nice if n and s did next and step when
>>> focusing
>>> a source window, but did nexti and stepi when focusing an asm window?
>>> When
>>> in an asm window, I think you will much more likely want to use
>>> nexti/stepi
>>> than next/step, so maybe they should be bound to the n and s rather than
>>> N
>>> and S in that case?
>>
>> This sounds like a great idea. If this is implemented, do you think N
>> and S are then useless and confusing (since n/s would be
>> focus-dependent while N/S won't)?
>
>
> I was thinking that perhaps that when focusing...
>
> a source view: s/n do source line stepping, S/N do assembly stepping
> an asm view: s/n do assembly stepping, S/N do source line stepping
>
> Basically, they would switch role.  Is that too confusing?
This might be OK, but what if current focus in on registers window?
Disable all single-key stepping?
>
>>>
>>> In order to get the patch in, you have two choices: if you plan to
>>> contribute more in the future, we can get you write access to the repo,
>>> so
>>> that you can push your patches in once they are approved.  Otherwise, we
>>> can
>>> push it for you.  Let me know what you prefer.
>>
>> Yes, I do plan to contribute in the future.
>
>
> I'll send you the details in private.
>
> Thanks,
>
> Simon
  

On 2017-07-27 11:11, Ruslan Kabatsayev wrote:
>> I was thinking that perhaps that when focusing...
>> 
>> a source view: s/n do source line stepping, S/N do assembly stepping
>> an asm view: s/n do assembly stepping, S/N do source line stepping
>> 
>> Basically, they would switch role.  Is that too confusing?
> This might be OK, but what if current focus in on registers window?
> Disable all single-key stepping?

Indeed.  And when the user is in the "Source + Asm + Prompt" 
configuration and the prompt is focused, what would we do?  That's 
perhaps a good idea in theory, but it would need to be more thought 
through.

You patch provides a predictable behavior across all modes, so I don't 
think we can go wrong with that.

Simon
  

Hello,

After using these shortcuts for some time I found that they are still
not best: one has to hold Shift in order to continuously step in
instruction mode. Would it be better to e.g. replace N->m, S->z? I
think we can basically use any non-occupied shortcuts here, they just
should be convenient, not necessarily be associated with "long"
version of the commands.

Regards,
Ruslan

On 27 July 2017 at 12:25, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> On 2017-07-27 11:11, Ruslan Kabatsayev wrote:
>>>
>>> I was thinking that perhaps that when focusing...
>>>
>>> a source view: s/n do source line stepping, S/N do assembly stepping
>>> an asm view: s/n do assembly stepping, S/N do source line stepping
>>>
>>> Basically, they would switch role.  Is that too confusing?
>>
>> This might be OK, but what if current focus in on registers window?
>> Disable all single-key stepping?
>
>
> Indeed.  And when the user is in the "Source + Asm + Prompt" configuration
> and the prompt is focused, what would we do?  That's perhaps a good idea in
> theory, but it would need to be more thought through.
>
> You patch provides a predictable behavior across all modes, so I don't think
> we can go wrong with that.
>
> Simon
  

On 2017-08-04 07:48, Ruslan Kabatsayev wrote:
> Hello,
> 
> After using these shortcuts for some time I found that they are still
> not best: one has to hold Shift in order to continuously step in
> instruction mode. Would it be better to e.g. replace N->m, S->z? I
> think we can basically use any non-occupied shortcuts here, they just
> should be convenient, not necessarily be associated with "long"
> version of the commands.
> 
> Regards,
> Ruslan

Ok, I understand how having to hold shift may not be so practical.

It would be nice if there was a pattern that made it easy to 
remember/deduce the keys for stepi/nexti from the keys for step/next, 
which are more obvious.  For example, if we used 'a' for stepi and 'b' 
for nexti, the "instruction" versions of the keys would both be at the 
left of their "non-instruction" counterpart.  That's with a 
QWERTY-centric view though, maybe it doesn't make sense with other 
layouts.

Simon
  

On Friday, August 04, 2017 03:29:01 PM Simon Marchi wrote:
> On 2017-08-04 07:48, Ruslan Kabatsayev wrote:
> > Hello,
> > 
> > After using these shortcuts for some time I found that they are still
> > not best: one has to hold Shift in order to continuously step in
> > instruction mode. Would it be better to e.g. replace N->m, S->z? I
> > think we can basically use any non-occupied shortcuts here, they just
> > should be convenient, not necessarily be associated with "long"
> > version of the commands.
> > 
> > Regards,
> > Ruslan
> 
> Ok, I understand how having to hold shift may not be so practical.
> 
> It would be nice if there was a pattern that made it easy to 
> remember/deduce the keys for stepi/nexti from the keys for step/next, 
> which are more obvious.  For example, if we used 'a' for stepi and 'b' 
> for nexti, the "instruction" versions of the keys would both be at the 
> left of their "non-instruction" counterpart.  That's with a 
> QWERTY-centric view though, maybe it doesn't make sense with other 
> layouts.

I agree it should be something deducible like a/b or d/m.
  

On 4 August 2017 at 16:29, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> On 2017-08-04 07:48, Ruslan Kabatsayev wrote:
>>
>> Hello,
>>
>> After using these shortcuts for some time I found that they are still
>> not best: one has to hold Shift in order to continuously step in
>> instruction mode. Would it be better to e.g. replace N->m, S->z? I
>> think we can basically use any non-occupied shortcuts here, they just
>> should be convenient, not necessarily be associated with "long"
>> version of the commands.
>>
>> Regards,
>> Ruslan
>
>
> Ok, I understand how having to hold shift may not be so practical.
>
> It would be nice if there was a pattern that made it easy to remember/deduce
> the keys for stepi/nexti from the keys for step/next, which are more
> obvious.  For example, if we used 'a' for stepi and 'b' for nexti, the
> "instruction" versions of the keys would both be at the left of their
> "non-instruction" counterpart.  That's with a QWERTY-centric view though,
> maybe it doesn't make sense with other layouts.
My choice of 'z' and 'm' was something like: 'z' is almost as 's', but
voiced, and 'm' is just next to 'n' both in the alphabet as well as on
QWERTY keyboard. I guess it's not too consistent nor memorizable.
I'll send another version, with a better choice, which I hope does make sense.
>
> Simon