Re: [PATCHv5 0/2] gdb: Change how frames are selected for 'frame' and 'info frame'.

[Andrew Burgess <andrew.burgess@embecosm.com>]

Ping!

Eli:

  In this message:
      https://sourceware.org/ml/gdb-patches/2018-07/msg00670.html
  Philippe highlighted that you might have some reservations about
  this patch series, which I think is currently the main blocker for
  this patch getting approval.

  In the thread started here:
      https://sourceware.org/ml/gdb-patches/2018-05/msg00299.html
  and ending here:
      https://sourceware.org/ml/gdb-patches/2018-06/msg00142.html
  you did review and approve one of the original patch variants, which
  is most like the "level" variant of the patch submitted here:
      https://sourceware.org/ml/gdb-patches/2018-08/msg00337.html

  I would be really grateful if you could let me know your current
  thoughts on this patch, are you happy to have the "level" variant
  merged based on your previous approval, or has you position changed?

  Thanks.

Philippe:

  Thanks for the review and feedback in:
      https://sourceware.org/ml/gdb-patches/2018-08/msg00349.html

  I wasn't sure if the second part of you mail was suggesting that
  those instances of level vs number needed to be resolved as part of
  this patch for you to be happy, or if this was just identifying what
  we should clean up if/when this patch is merged.  Additional clarity
  here would help me figure out my next steps for getting this patch
  merged.

  Thanks.

Many thanks to everyone who's taken time to review this for me.

Thanks,
Andrew



* Andrew Burgess <andrew.burgess@embecosm.com> [2018-08-13 23:20:09 +0100]:

> Given a lack of feedback on v4, I've put together two alternative
> versions to pick between...
> 
>   (A) In this version I took onboard the feedback from Eli and
>       Philippe that using "level" might be confusing to users, as
>       "number" is historically what has been used for the integer
>       label we give to frames.
> 
>       The command for selecting a frame by integer is now, 'frame
>       number <NUMBER>' and almost all references to "level" have now
>       been removed from the patch.
> 
>   (B) In this version I've taken onboard the advice from Philippe,
>       identifying places in the patch where I was mixing use of
>       "level" and "number".  I've doubled down on "level" and removed
>       most uses of "number" from the patch.
> 
> If we select patch 'A' then me might want to update 'frame apply level
> ...' to 'frame apply number ...' for consistency.
> 
> If we select patch 'B' then we will probably need a follow up patch
> that goes through the documentation to tighten up references to
> "number" vs "level".
> 
> I really don't mind which approach we take, I guess I'd probably pick
> 'B' over 'A' given we already have 'frame apply level ...' in GDB, but
> if there's preference for 'A' then that's fine.
> 
> I'm also happy to do the follow up patches once we have some agreement
> on which way to go.
> 
> Thanks,
> Andrew
> 
> ---
> 
> Andrew Burgess (1):
>   gdb: Change how frames are selected for 'frame' and 'info frame'.
> 
>  gdb/ChangeLog                               |  36 ++
>  gdb/NEWS                                    |   8 +
>  gdb/cli/cli-decode.c                        |  44 ++-
>  gdb/command.h                               |  14 +
>  gdb/doc/ChangeLog                           |   8 +
>  gdb/doc/gdb.texinfo                         | 108 ++++--
>  gdb/mi/mi-cmd-stack.c                       |   4 +-
>  gdb/stack.c                                 | 535 +++++++++++++++++++---------
>  gdb/stack.h                                 |   2 +-
>  gdb/testsuite/ChangeLog                     |   7 +
>  gdb/testsuite/gdb.base/frame-selection.c    |  52 +++
>  gdb/testsuite/gdb.base/frame-selection.exp  | 157 ++++++++
>  gdb/testsuite/gdb.mi/mi-frame-selection.c   |  34 ++
>  gdb/testsuite/gdb.mi/mi-frame-selection.exp |  89 +++++
>  14 files changed, 896 insertions(+), 202 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/frame-selection.c
>  create mode 100644 gdb/testsuite/gdb.base/frame-selection.exp
>  create mode 100644 gdb/testsuite/gdb.mi/mi-frame-selection.c
>  create mode 100644 gdb/testsuite/gdb.mi/mi-frame-selection.exp
> 
> -- 
> 2.14.4
>