Message ID | 1499926070-13827-1-git-send-email-b7.10110111@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 102093 invoked by alias); 13 Jul 2017 06:13:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 69757 invoked by uid 89); 13 Jul 2017 06:11:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=H*RU:209.85.215.68, Hx-spam-relays-external:209.85.215.68 X-HELO: mail-lf0-f68.google.com Received: from mail-lf0-f68.google.com (HELO mail-lf0-f68.google.com) (209.85.215.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 13 Jul 2017 06:08:05 +0000 Received: by mail-lf0-f68.google.com with SMTP id g21so4890223lfk.1 for <gdb-patches@sourceware.org>; Wed, 12 Jul 2017 23:08:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=wu6m9jNwqBjeAH9THMG3yNaVzaM9yVNC8QxAcPx7wRA=; b=QPvHQxY6cqBa4KDcUBWQHcLDIu75wogOLfeitYUWp2tObSuq7Vpjndwc4VCQwuXzDy VfDiyJZs1FMrYKH005noO8ADaW0UqQ/BDPx5r7khj98eIAIhq5+1MkOBjCfVSiOX3FCR r7AX2cR7YduGcctWAdY8IFygZht3y1vb2U44PmaEfgX03j1CprxwKxTY6+EBO3ypqUvC WRHRCVpAQGjPPcvDe1V8oVMX8NZQYwlDWinpw6sAist+uWAamEDHyMky1VxOrlbktHhc MLubM+XUl0krwar/oxgilL1F8o62+fyzgVfsITDQf0ZouHfHY+L34xQXVW7AaT8ToI+V stPw== X-Gm-Message-State: AIVw1133Urxc/vqfqtEy2AXBLit/fH00DDxDmlegi4XTONiI5oDLC3er 2C/x9msdcAzqdZTg X-Received: by 10.25.56.13 with SMTP id f13mr675230lfa.167.1499926082753; Wed, 12 Jul 2017 23:08:02 -0700 (PDT) Received: from localhost.localdomain ([91.215.122.25]) by smtp.gmail.com with ESMTPSA id 28sm926249ljw.33.2017.07.12.23.08.01 (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 12 Jul 2017 23:08:01 -0700 (PDT) From: Ruslan Kabatsayev <b7.10110111@gmail.com> To: gdb-patches@sourceware.org Cc: Ruslan Kabatsayev <b7.10110111@gmail.com> Subject: [PATCH] Add shortcuts for "nexti" and "stepi" commands in Single-Key mode Date: Thu, 13 Jul 2017 09:07:50 +0300 Message-Id: <1499926070-13827-1-git-send-email-b7.10110111@gmail.com> X-IsSubscribed: yes |
Commit Message
Ruslan Kabatsayev
July 13, 2017, 6:07 a.m. UTC
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(+)
Comments
> 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
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" },