bug#59730: gdb-mi.el: Local variables reordering

bug#59730: gdb-mi.el: Local variables reordering

[Gustaf Waldemarson]

[-- Attachment #1.1: Type: text/plain, Size: 12815 bytes --]

Hello,

After about 3 years, I finally spent some time to actually change the order
of local variables as I had originally suggest in an old feature
(bug#34589).

In summary, this patch does this: In gdb-mi.el mode, for local C/C++
variables that were previously written out:

- | type | name | value|

Now write them out as:

- | name | type | value |

Additionally, cap the string length of the name and type to
`gdb-locals-max-name-length` and `gdb-locals-max-type-length` respectively
(new custom variables with a default set to 20). I also changed the table
to always left-align the values when we're printing the locals.

Turns out it was really easy to fix, but I may have missed some subtleties,
so feel free to give it a look or start a discussion whether this is a good
idea or not. I personally prefer it this way since it is much easier and
faster to see the values of individual variables, especially when the
type-info get very long.

Best regards,
Gustaf

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.33, cairo version 1.16.0) of 2022-11-30 built on ShadowX
Repository revision: ec759e82b39f6ee54e9187acc9525cd65d704c59
Repository branch: master
System Description: Ubuntu 22.04.1 LTS

Configured using:
 'configure --prefix=/home/xaldew/.local
 '--program-transform-name=s/^ctags$/ctags.emacs/' --without-makeinfo
 --with-xpm=ifavailable --with-jpeg=ifavailable --with-gif=ifavailable
 --with-tiff=ifavailable'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG LCMS2
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND
THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: sv_SE.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: C++//l

Minor modes in effect:
  lsp-diagnostics-mode: t
  lsp-headerline-breadcrumb-mode: t
  lsp-modeline-workspace-status-mode: t
  lsp-modeline-diagnostics-mode: t
  lsp-modeline-code-actions-mode: t
  dap-tooltip-mode: t
  dap-ui-many-windows-mode: t
  dap-ui-controls-mode: t
  dap-ui-mode: t
  gdb-many-windows: t
  beacon-mode: t
  modern-c++-font-lock-mode: t
  lsp-completion-mode: t
  treemacs-filewatch-mode: t
  treemacs-follow-mode: t
  treemacs-git-mode: t
  treemacs-fringe-indicator-mode: t
  dap-auto-configure-mode: t
  dap-mode: t
  flyspell-mode: t
  whitespace-mode: t
  rainbow-delimiters-mode: t
  hes-mode: t
  projectile-mode: t
  lsp-ui-mode: t
  lsp-ui-sideline-mode: t
  flycheck-mode: t
  lsp-managed-mode: t
  lsp-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  global-company-mode: t
  company-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-anzu-mode: t
  anzu-mode: t
  perspeen-mode: t
  windmove-mode: t
  which-key-mode: t
  anyclip-mode: t
  override-global-mode: t
  cwarn-mode: t
  electric-pair-mode: t
  save-place-mode: t
  global-subword-mode: t
  subword-mode: t
  winner-mode: t
  global-auto-revert-mode: t
  xterm-mouse-mode: t
  savehist-mode: t
  ido-everywhere: t
  gud-tooltip-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  auto-fill-function: yas--auto-fill
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  abbrev-mode: t

Load-path shadows:
/home/xaldew/.config/emacs/elpa/transient-20221130.1823/transient hides
/home/xaldew/.local/share/emacs/30.0.50/lisp/transient

Features:
(shadow sort bbdb-message guess-language mail-extr warnings emacsbug
message yank-media dired dired-loaddefs rfc822 mml mml-sec epa derived
gnus-util time-date mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils display-line-numbers server pinentry
files-x lsp-diagnostics lsp-headerline lsp-icons lsp-modeline dap-mouse
dap-ui gdb-mi bui bui-list bui-info bui-entry bui-core bui-history
bui-button bui-utils lsp-lens beacon vc-git diff-mode vc-dispatcher
modern-cpp-font-lock .yas-setup.el cc-mode/.yas-setup.el view lsp-zig
lsp-steep lsp-svelte lsp-sqls lsp-ruby-syntax-tree lsp-yaml lsp-xml
lsp-vimscript lsp-vhdl lsp-volar lsp-vetur lsp-html lsp-verilog lsp-vala
lsp-v lsp-typeprof lsp-ttcn3 lsp-toml lsp-terraform lsp-tex lsp-sorbet
lsp-solargraph lsp-rust lsp-rf lsp-remark lsp-racket lsp-r
lsp-purescript lsp-pylsp lsp-pyls lsp-pwsh lsp-php lsp-pls
lsp-perlnavigator lsp-perl lsp-openscad lsp-ocaml lsp-magik lsp-nix
lsp-nim lsp-nginx lsp-mint lsp-marksman lsp-markdown lsp-lua lsp-ltex
lsp-kotlin lsp-json lsp-javascript lsp-idris lsp-haxe lsp-groovy
lsp-hack lsp-graphql lsp-gleam lsp-go lsp-completion lsp-gdscript
lsp-fsharp lsp-fortran lsp-eslint lsp-erlang lsp-emmet lsp-elixir
lsp-elm lsp-dockerfile lsp-dhall lsp-d lsp-css lsp-csharp gnutls
lsp-crystal lsp-cmake lsp-clojure lsp-treemacs lsp-treemacs-generic
lsp-treemacs-themes treemacs-treelib treemacs treemacs-header-line
treemacs-compatibility treemacs-mode treemacs-interface
treemacs-persistence treemacs-filewatch-mode treemacs-follow-mode
treemacs-rendering treemacs-annotations treemacs-async
treemacs-workspaces treemacs-dom treemacs-visuals
treemacs-fringe-indicator treemacs-scope pulse treemacs-faces
treemacs-icons treemacs-themes treemacs-core-utils pfuture hl-line
treemacs-logging treemacs-customization treemacs-macros
lsp-semantic-tokens lsp-clangd lsp-beancount lsp-bash lsp-astro
lsp-ansible lsp-angular lsp-ada lsp-actionscript dap-gdb-lldb dap-utils
dom xml dap-mode dap-tasks dap-launch lsp-docker yaml posframe
dap-overlays term/tmux term/xterm xterm form-feed paredit nameless
flyspell ispell whitespace rainbow-delimiters highlight-escape-sequences
projectile lisp-mnt grep ibuf-ext ibuffer ibuffer-loaddefs lsp-ui
lsp-ui-flycheck lsp-ui-doc goto-addr lsp-ui-imenu lsp-ui-peek
lsp-ui-sideline flycheck lsp-mode tree-widget spinner network-stream
puny nsm markdown-mode noutline outline inline imenu f f-shortdoc
shortdoc ewoc epg rfc6068 epg-config compile text-property-search
lsp-ui-util face-remap find-func lsp-protocol s ht dash py-snippets
yasnippet-radical-snippets yasnippet-snippets yasnippet company-oddmuse
company-keywords company-etags etags fileloop generator xref project
company-gtags company-dabbrev-code company-dabbrev company-files
company-clang company-capf company-cmake company-semantic
company-template company-bbdb company undo-tree diff queue anzu
thingatpt atomic-chrome websocket bindat let-alist
color-theme-approximate perspeen perspeen-tab powerline advice
powerline-separators color powerline-themes delim-col hydra-examples
windmove rect hydra lv bbdb bbdb-site timezone cus-edit pp cus-load
icons wid-edit ace-link avy pcase which-key anyclip-mode cl-extra
help-mode edmacro kmacro diminish use-package use-package-ensure
use-package-delight use-package-diminish use-package-bind-key bind-key
easy-mmode use-package-core finder-inf local-autoloads cwarn cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs elec-pair saveplace cap-words superword subword winner
autorevert filenotify xt-mouse tango-dark-theme savehist ido gud comint
ansi-osc ansi-color ring keybinds terminals ace-link-autoloads
unfill-autoloads company-auctex-autoloads gl-conf-mode-autoloads
spacemacs-theme-autoloads css-eldoc-autoloads beacon-autoloads
emms-autoloads paredit-autoloads glsl-mode-autoloads
highlight-escape-sequences-autoloads evil-autoloads powershell-autoloads
gnuplot-mode-autoloads cuda-mode-autoloads ein-autoloads
debbugs-autoloads ssh-config-mode-autoloads gnuplot-autoloads
perspeen-autoloads cider-autoloads sesman-autoloads
clojure-mode-autoloads dart-mode-autoloads sublime-themes-autoloads
coffee-mode-autoloads opencl-mode-autoloads json-mode-autoloads rx
company-anaconda-autoloads pinentry-autoloads rainbow-mode-autoloads
htmlize-autoloads flycheck-rust-autoloads kotlin-mode-autoloads
lsp-ltex-autoloads bbdb-vcard-autoloads ebdb-autoloads
projectile-autoloads jira-markup-mode-autoloads
browse-kill-ring-autoloads slime-autoloads macrostep-autoloads
helm-dash-autoloads dap-mode-autoloads lsp-docker-autoloads
yaml-autoloads lsp-treemacs-autoloads treemacs-autoloads cfrs-autoloads
pfuture-autoloads bui-autoloads sx-autoloads jenkins-autoloads
smart-jump-autoloads bbdb-autoloads parseedn-autoloads delight-autoloads
anzu-autoloads x86-lookup-autoloads undo-tree-autoloads queue-autoloads
langtool-autoloads parseclj-autoloads plantuml-mode-autoloads
git-modes-autoloads ace-window-autoloads go-mode-autoloads
powerline-autoloads lsp-ui-autoloads deferred-autoloads iedit-autoloads
csv-mode-autoloads srefactor-autoloads abc-mode-autoloads autoinsert
atomic-chrome-autoloads solarized-theme-autoloads
anaconda-mode-autoloads web-mode-autoloads flycheck-kotlin-autoloads
lua-mode-autoloads nov-autoloads dash-docs-autoloads
rainbow-delimiters-autoloads form-feed-autoloads
yasnippet-radical-snippets-autoloads cmake-mode-autoloads
powerthesaurus-autoloads request-autoloads pyimport-autoloads
shut-up-autoloads json-snatcher-autoloads helm-lsp-autoloads
lsp-mode-autoloads spinner-autoloads ht-autoloads cargo-autoloads
clang-format-autoloads toml-mode-autoloads diminish-autoloads
websocket-autoloads yasnippet-snippets-autoloads nameless-autoloads
cdlatex-autoloads eclim-autoloads anaphora-autoloads rust-mode-autoloads
py-snippets-autoloads magit-gerrit-autoloads rmsbolt-autoloads
magit-svn-autoloads pythonic-autoloads company-quickhelp-autoloads
magit-annex-autoloads goto-chg-autoloads poly-markdown-autoloads
markdown-mode-autoloads polymode-autoloads alert-autoloads
gntp-autoloads log4e-autoloads color-theme-approximate-autoloads
use-package-autoloads bind-key-autoloads calfw-autoloads
graphviz-dot-mode-autoloads flycheck-package-autoloads
package-lint-autoloads flycheck-autoloads pkg-info-autoloads
epl-autoloads ecb-autoloads avy-autoloads modern-cpp-font-lock-autoloads
dts-mode-autoloads zenburn-theme-autoloads yaml-mode-autoloads
esxml-autoloads kv-autoloads elpy-autoloads pyvenv-autoloads
highlight-indentation-autoloads ob-ipython-autoloads f-autoloads
dash-functional-autoloads hydra-autoloads lv-autoloads
auto-complete-auctex-autoloads auto-complete-autoloads
yasnippet-autoloads auctex-latexmk-autoloads auctex-autoloads tex-site
ahk-mode-autoloads magit-autoloads magit-section-autoloads
git-commit-autoloads with-editor-autoloads transient-autoloads
compat-autoloads cask-mode-autoloads company-c-headers-autoloads
pos-tip-autoloads zerodark-theme-autoloads all-the-icons-autoloads
image+-autoloads expand-region-autoloads gnus-desktop-notify-autoloads
helm-autoloads helm-core-autoloads async-autoloads ggtags-autoloads
mmm-mode-autoloads posframe-autoloads company-math-autoloads
math-symbol-lists-autoloads company-autoloads ivy-autoloads
uimage-autoloads guess-language-autoloads dumb-jump-autoloads
popup-autoloads info dash-autoloads s-autoloads google-c-style-autoloads
which-key-autoloads package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie generate-lisp-file
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv
bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl tooltip
cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 465686 22927)
 (symbols 48 41605 1)
 (strings 32 144416 7236)
 (string-bytes 1 5047896)
 (vectors 16 74684)
 (vector-slots 8 1033911 12667)
 (floats 8 419 1011)
 (intervals 56 2262 292)
 (buffers 984 17))

[-- Attachment #1.2: Type: text/html, Size: 13953 bytes --]

[-- Attachment #2: 0001-gdb-mi.el-Swap-type-and-name-column-in-locals.patch --]
[-- Type: application/x-patch, Size: 1921 bytes --]

bug#59730: gdb-mi.el: Local variables reordering

[Eli Zaretskii]

> From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> Date: Wed, 30 Nov 2022 23:09:14 +0100
> 
> In summary, this patch does this: In gdb-mi.el mode, for local C/C++ variables that were previously written
> out:
> 
> - | type | name | value|
> 
> Now write them out as:
> 
> - | name | type | value |
> 
> Additionally, cap the string length of the name and type to `gdb-locals-max-name-length` and
> `gdb-locals-max-type-length` respectively (new custom variables with a default set to 20). I also changed the
> table to always left-align the values when we're printing the locals.
> 
> Turns out it was really easy to fix, but I may have missed some subtleties, so feel free to give it a look or
> start a discussion whether this is a good idea or not. I personally prefer it this way since it is much easier
> and faster to see the values of individual variables, especially when the type-info get very long.

First, if the problem is that the type names are long, maybe it will be
enough to truncate them without changing the order?

Also, latest version of GDB allow control on which types get shown in full
and which are shown as <...> -- did you try to use that GDB option to make
the display more easily readable?

And wouldn't it be better to truncate the string with
truncate-string-to-width or with string-truncate-left instead?

And finally, when the type is truncated, would it be possible to add a
tooltip with the full name of the type, so that users who need that could
hover the mouse above the truncated type and see it in full?

Thanks.

bug#59730: gdb-mi.el: Local variables reordering

[Gustaf Waldemarson]

[-- Attachment #1: Type: text/plain, Size: 3588 bytes --]

> First, if the problem is that the type names are long, maybe it will be
> enough to truncate them without changing the order?

The truncation is the main issue, although I would argue that this is also
a good opportunity to change
the order as well given some of the benefits of a left-to-right reading
order, (e.g., it is easier to read and
parse, as explained Herb Sutter^1). Other languages such as Rust/Go could
also benefit from this. That said,
I guess this might be a western thing, and it is hardly a hill I would die
on, so to speak.

> Also, latest version of GDB allow control on which types get shown in full
> and which are shown as <...> -- did you try to use that GDB option to make
> the display more easily readable?

I was not aware of this. I will look into it and see if I can enable it.
That said, there's probably benefits to having
the option to truncate from Emacs anyways, primarily to support older
versions of GDB.

> And wouldn't it be better to truncate the string with
> truncate-string-to-width or with string-truncate-left instead?

Absolutely, I just was not aware of these (better) tools. One could even
argue that a custom filter-function could be warranted,
but I think that's a bit overkill right now at least.

> And finally, when the type is truncated, would it be possible to add a
> tooltip with the full name of the type, so that users who need that could
> hover the mouse above the truncated type and see it in full?

Excellent idea, that should be doable by just adding some properties to the
strings, I'll see if I can sort these things out as well.

Thanks for the feedback, I'll start working on an updated patch soon-ish

^1. See e.g.
https://softwareengineering.stackexchange.com/questions/101978/advantages-of-a-left-to-right-language-syntax
^1. and https://herbsutter.com/2011/05/04/interview-on-channel-9-2/

Best regards,
Gustaf

Den tors 1 dec. 2022 kl 16:45 skrev Eli Zaretskii <eliz@gnu.org>:

> > From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> > Date: Wed, 30 Nov 2022 23:09:14 +0100
> >
> > In summary, this patch does this: In gdb-mi.el mode, for local C/C++
> variables that were previously written
> > out:
> >
> > - | type | name | value|
> >
> > Now write them out as:
> >
> > - | name | type | value |
> >
> > Additionally, cap the string length of the name and type to
> `gdb-locals-max-name-length` and
> > `gdb-locals-max-type-length` respectively (new custom variables with a
> default set to 20). I also changed the
> > table to always left-align the values when we're printing the locals.
> >
> > Turns out it was really easy to fix, but I may have missed some
> subtleties, so feel free to give it a look or
> > start a discussion whether this is a good idea or not. I personally
> prefer it this way since it is much easier
> > and faster to see the values of individual variables, especially when
> the type-info get very long.
>
> First, if the problem is that the type names are long, maybe it will be
> enough to truncate them without changing the order?
>
> Also, latest version of GDB allow control on which types get shown in full
> and which are shown as <...> -- did you try to use that GDB option to make
> the display more easily readable?
>
> And wouldn't it be better to truncate the string with
> truncate-string-to-width or with string-truncate-left instead?
>
> And finally, when the type is truncated, would it be possible to add a
> tooltip with the full name of the type, so that users who need that could
> hover the mouse above the truncated type and see it in full?
>
> Thanks.
>

[-- Attachment #2: Type: text/html, Size: 4612 bytes --]

bug#59730: gdb-mi.el: Local variables reordering

[Gustaf Waldemarson]

[-- Attachment #1.1: Type: text/plain, Size: 4116 bytes --]

The updated patch is attached to this mail. Although I was not able to find
any reference to the GDB options that changes
how types are displayed (
https://sourceware.org/gdb/onlinedocs/gdb/Print-Settings.html#Print-Settings).
Am I just
blind? Or did you have a specific setting in mind?

Best regards,
Gustaf

Den tors 1 dec. 2022 kl 21:40 skrev Gustaf Waldemarson <
gustaf.waldemarson@gmail.com>:

> > First, if the problem is that the type names are long, maybe it will be
> > enough to truncate them without changing the order?
>
> The truncation is the main issue, although I would argue that this is also
> a good opportunity to change
> the order as well given some of the benefits of a left-to-right reading
> order, (e.g., it is easier to read and
> parse, as explained Herb Sutter^1). Other languages such as Rust/Go could
> also benefit from this. That said,
> I guess this might be a western thing, and it is hardly a hill I would die
> on, so to speak.
>
> > Also, latest version of GDB allow control on which types get shown in
> full
> > and which are shown as <...> -- did you try to use that GDB option to
> make
> > the display more easily readable?
>
> I was not aware of this. I will look into it and see if I can enable it.
> That said, there's probably benefits to having
> the option to truncate from Emacs anyways, primarily to support older
> versions of GDB.
>
> > And wouldn't it be better to truncate the string with
> > truncate-string-to-width or with string-truncate-left instead?
>
> Absolutely, I just was not aware of these (better) tools. One could even
> argue that a custom filter-function could be warranted,
> but I think that's a bit overkill right now at least.
>
> > And finally, when the type is truncated, would it be possible to add a
> > tooltip with the full name of the type, so that users who need that could
> > hover the mouse above the truncated type and see it in full?
>
> Excellent idea, that should be doable by just adding some properties to
> the strings, I'll see if I can sort these things out as well.
>
> Thanks for the feedback, I'll start working on an updated patch soon-ish
>
> ^1. See e.g.
> https://softwareengineering.stackexchange.com/questions/101978/advantages-of-a-left-to-right-language-syntax
> ^1. and https://herbsutter.com/2011/05/04/interview-on-channel-9-2/
>
> Best regards,
> Gustaf
>
> Den tors 1 dec. 2022 kl 16:45 skrev Eli Zaretskii <eliz@gnu.org>:
>
>> > From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
>> > Date: Wed, 30 Nov 2022 23:09:14 +0100
>> >
>> > In summary, this patch does this: In gdb-mi.el mode, for local C/C++
>> variables that were previously written
>> > out:
>> >
>> > - | type | name | value|
>> >
>> > Now write them out as:
>> >
>> > - | name | type | value |
>> >
>> > Additionally, cap the string length of the name and type to
>> `gdb-locals-max-name-length` and
>> > `gdb-locals-max-type-length` respectively (new custom variables with a
>> default set to 20). I also changed the
>> > table to always left-align the values when we're printing the locals.
>> >
>> > Turns out it was really easy to fix, but I may have missed some
>> subtleties, so feel free to give it a look or
>> > start a discussion whether this is a good idea or not. I personally
>> prefer it this way since it is much easier
>> > and faster to see the values of individual variables, especially when
>> the type-info get very long.
>>
>> First, if the problem is that the type names are long, maybe it will be
>> enough to truncate them without changing the order?
>>
>> Also, latest version of GDB allow control on which types get shown in full
>> and which are shown as <...> -- did you try to use that GDB option to make
>> the display more easily readable?
>>
>> And wouldn't it be better to truncate the string with
>> truncate-string-to-width or with string-truncate-left instead?
>>
>> And finally, when the type is truncated, would it be possible to add a
>> tooltip with the full name of the type, so that users who need that could
>> hover the mouse above the truncated type and see it in full?
>>
>> Thanks.
>>
>

[-- Attachment #1.2: Type: text/html, Size: 5468 bytes --]

[-- Attachment #2: 0001-gdb-mi.el-Swap-type-and-name-column-in-locals.patch --]
[-- Type: text/x-patch, Size: 2142 bytes --]

From 4f238c72acbc15eaee7ed7cd19c3aeac19a55a7b Mon Sep 17 00:00:00 2001
From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
Date: Tue, 29 Nov 2022 23:40:23 +0100
Subject: [PATCH] gdb-mi.el: Swap type and name column in locals.

Additionally, truncate the column lengths and add the full length as a
help-text (tooltip).
---
 lisp/progmodes/gdb-mi.el | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index e8d8f9104e4..197d133f0ee 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -4355,6 +4355,19 @@ gdb-locals-value-limit
   :group 'gud
   :version "29.1")
 
+(defcustom gdb-locals-max-type-length 20
+  "Maximum number of character to display in the local variables type column."
+  :type 'integer
+  :group 'gud
+  :version "30.0")
+
+(defcustom gdb-locals-max-name-length 20
+  "Maximum number of character to display in the local variables name column."
+  :type 'integer
+  :group 'gud
+  :version "30.0")
+
+
 (defvar gdb-locals-values-table (make-hash-table :test #'equal)
   "Mapping of local variable names to a string with their value.")
 
@@ -4431,11 +4444,16 @@ gdb-locals-handler-custom
                                             help-echo "mouse-2: edit value"
                                             local-map ,gdb-edit-locals-map-1)
                                value))
+        (setf (gdb-table-right-align table) t)
         (gdb-table-add-row
          table
          (list
-          (propertize type 'font-lock-face font-lock-type-face)
-          (propertize name 'font-lock-face font-lock-variable-name-face)
+          (propertize (string-truncate-left name gdb-locals-max-name-length)
+                      'font-lock-face font-lock-variable-name-face
+                      'help-echo name)
+          (propertize (string-truncate-left type gdb-locals-max-type-length)
+                      'font-lock-face font-lock-type-face
+                      'help-echo type)
           value)
          `(gdb-local-variable ,local))))
     (insert (gdb-table-string table " "))
-- 
2.34.1

bug#59730: gdb-mi.el: Local variables reordering

[Eli Zaretskii]

> From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> Date: Thu, 1 Dec 2022 22:53:26 +0100
> Cc: 59730@debbugs.gnu.org
> 
> The updated patch is attached to this mail. Although I was not able to find any reference to the GDB options
> that changes
> how types are displayed (https://sourceware.org/gdb/onlinedocs/gdb/Print-Settings.html#Print-Settings). Am
> I just
> blind? Or did you have a specific setting in mind?

I meant "set print frame-arguments", but maybe it is not relevant to what
you are trying to change.

I will review the patch soon, thanks.

bug#59730: gdb-mi.el: Local variables reordering

[Gustaf Waldemarson]

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

Ahh, I see. I could be wrong, but I think that option would only change the
display of the value of local variables,
not the type of them. Anyways, thanks for the feedback and for considering
the patch!

Best regards,
Gustaf

Den fre 2 dec. 2022 kl 14:49 skrev Eli Zaretskii <eliz@gnu.org>:

> > From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> > Date: Thu, 1 Dec 2022 22:53:26 +0100
> > Cc: 59730@debbugs.gnu.org
> >
> > The updated patch is attached to this mail. Although I was not able to
> find any reference to the GDB options
> > that changes
> > how types are displayed (
> https://sourceware.org/gdb/onlinedocs/gdb/Print-Settings.html#Print-Settings).
> Am
> > I just
> > blind? Or did you have a specific setting in mind?
>
> I meant "set print frame-arguments", but maybe it is not relevant to what
> you are trying to change.
>
> I will review the patch soon, thanks.
>

[-- Attachment #2: Type: text/html, Size: 1550 bytes --]

bug#59730: gdb-mi.el: Local variables reordering

[Eli Zaretskii]

> From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> Date: Thu, 1 Dec 2022 22:53:26 +0100
> Cc: 59730@debbugs.gnu.org
> 
> The updated patch is attached to this mail.

Thanks.  I have a couple of minor comments:

> +(defcustom gdb-locals-max-type-length 20
> +  "Maximum number of character to display in the local variables type column."
> +  :type 'integer
> +  :group 'gud
> +  :version "30.0")

This should be "30.1".  We don't release NN.o versions of Emacs.

> +(defcustom gdb-locals-max-name-length 20
> +  "Maximum number of character to display in the local variables name column."
> +  :type 'integer
> +  :group 'gud
> +  :version "30.0")

Likewise.

Also, please add more detailed description of what these options control.
The first line of the doc string is fine, but please add more text
explaining that this setting is for displaying the variables in the "Locals"
buffer.

> -          (propertize type 'font-lock-face font-lock-type-face)
> -          (propertize name 'font-lock-face font-lock-variable-name-face)
> +          (propertize (string-truncate-left name gdb-locals-max-name-length)
> +                      'font-lock-face font-lock-variable-name-face
> +                      'help-echo name)
> +          (propertize (string-truncate-left type gdb-locals-max-type-length)
> +                      'font-lock-face font-lock-type-face
> +                      'help-echo type)

I'm uncomfortable with backward-incompatible change in behavior for which
there's no way of getting the old behavior.  So please add another defcustom
that could be used to get the original order of type/name/value.

There also should be a NEWS entry about these new options, and the text
there should explain how to get back old behavior before these changes.

bug#59730: gdb-mi.el: Local variables reordering

[Gustaf Waldemarson]

[-- Attachment #1.1: Type: text/plain, Size: 3553 bytes --]

> I'm uncomfortable with backward-incompatible change in behavior for which
> there's no way of getting the old behavior.  So please add another
defcustom
> that could be used to get the original order of type/name/value.

> There also should be a NEWS entry about these new options, and the text
> there should explain how to get back old behavior before these changes.

Hmm, this turned out to be a bit harder change, and I did not particularly
like
the thought of having 3 defcustoms control this behavior. So I spent a bit
of time
to refactor things a bit.

So, this new version only uses a single defcustom,
`gdb-locals-table-row-config',
which is an alist the user can change to specify the truncation and order
of the
local variable properties (i.e., name, type, value).

I have set this to '((name . 20)(type . 20)(value . 100)), so to get the
old behavior
one would change this to '(type . 0)(name . 0)(value . 0).

Additionally, I found a minor bug in `gdb-locals-value-filter' which did
not work
properly when the value was nil. There I also noticed that that the
`gdb-locals-value-limit'
has become redundant with this change. Unfortunately, I wasn't able to
figure out how
to mark that defcustom as deprecated, is there some kind of macro that
should be done
for that, or should I just remove it?

I also added content to the `etc/NEWS`, as requested but I haven't actually
done that
before, so I may very well have messed up the formatting, so feel free to
double check
things there

Best regards,
Gustaf


Den lör 3 dec. 2022 kl 20:59 skrev Eli Zaretskii <eliz@gnu.org>:

> > From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> > Date: Thu, 1 Dec 2022 22:53:26 +0100
> > Cc: 59730@debbugs.gnu.org
> >
> > The updated patch is attached to this mail.
>
> Thanks.  I have a couple of minor comments:
>
> > +(defcustom gdb-locals-max-type-length 20
> > +  "Maximum number of character to display in the local variables type
> column."
> > +  :type 'integer
> > +  :group 'gud
> > +  :version "30.0")
>
> This should be "30.1".  We don't release NN.o versions of Emacs.
>
> > +(defcustom gdb-locals-max-name-length 20
> > +  "Maximum number of character to display in the local variables name
> column."
> > +  :type 'integer
> > +  :group 'gud
> > +  :version "30.0")
>
> Likewise.
>
> Also, please add more detailed description of what these options control.
> The first line of the doc string is fine, but please add more text
> explaining that this setting is for displaying the variables in the
> "Locals"
> buffer.
>
> > -          (propertize type 'font-lock-face font-lock-type-face)
> > -          (propertize name 'font-lock-face font-lock-variable-name-face)
> > +          (propertize (string-truncate-left name
> gdb-locals-max-name-length)
> > +                      'font-lock-face font-lock-variable-name-face
> > +                      'help-echo name)
> > +          (propertize (string-truncate-left type
> gdb-locals-max-type-length)
> > +                      'font-lock-face font-lock-type-face
> > +                      'help-echo type)
>
> I'm uncomfortable with backward-incompatible change in behavior for which
> there's no way of getting the old behavior.  So please add another
> defcustom
> that could be used to get the original order of type/name/value.
>
> There also should be a NEWS entry about these new options, and the text
> there should explain how to get back old behavior before these changes.
>

[-- Attachment #1.2: Type: text/html, Size: 4628 bytes --]

[-- Attachment #2: 0001-gdb-mi.el-Configure-variable-order-and-length.patch --]
[-- Type: text/x-patch, Size: 5142 bytes --]

From e65276fb92a1e7a86d2ce894482eaf5c53b7a34e Mon Sep 17 00:00:00 2001
From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
Date: Tue, 29 Nov 2022 23:40:23 +0100
Subject: [PATCH] gdb-mi.el: Configure variable order and length.

This changes allows users to configure the order of various properties
as well as truncating their length. The full description of each
property is available as a help-text (tooltip).
---
 etc/NEWS                 | 17 +++++++++++++
 lisp/progmodes/gdb-mi.el | 53 ++++++++++++++++++++++++++++++++--------
 2 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 9b8edde5155..473d427e757 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -40,6 +40,23 @@ connection.
 
 \f
 * Changes in Specialized Modes and Packages in Emacs 30.1
+---
+** Variable order and truncation can now be configured in gdb-many-window mode.
+The new variable `gdb-locals-table-row-config' allows users to
+configure the order and max length of various properties in the local
+variables frame.
+
+By default, this variable is set to write the properties in the order:
+name, type and value. Where name and type are truncated to 20
+characters, and the value is truncated to 100.
+
+In order to restore the old display behavior, set
+`gdb-locals-table-row-config' to '((type . 0)(name . 0)(value . 0)).
+
+Additionally, this behavior encompasses the actions of the old
+`gdb-locals-value-limit' variable, which is no longer used and marked
+deprecated.
+
 
 \f
 * New Modes and Packages in Emacs 30.1
diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index e8d8f9104e4..80950fec77e 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -4355,6 +4355,24 @@ gdb-locals-value-limit
   :group 'gud
   :version "29.1")
 
+(defcustom gdb-locals-table-row-config '((name . 20)
+                                         (type . 20)
+                                         (value . 100))
+  "Configuration for table rows in the local variable display.
+
+An alist that controls the display of the name, type and value of
+local variables inside the currently active stack-frame.  The key
+controls which column to change whereas the value determines the
+maximum number of characters to display in each column.  A value
+of 0 means there is no limit.
+
+Additionally, the order the element in the alist determines the
+left-to-right display order of the properties."
+  :type '(alist :key-type 'symbol :value-type 'integer)
+  :group 'gud
+  :version "30.1")
+
+
 (defvar gdb-locals-values-table (make-hash-table :test #'equal)
   "Mapping of local variable names to a string with their value.")
 
@@ -4384,12 +4402,9 @@ gdb-edit-locals-map-1
 
 (defun gdb-locals-value-filter (value)
   "Filter function for the local variable VALUE."
-  (let* ((no-nl (replace-regexp-in-string "\n" " " value))
-         (str (replace-regexp-in-string "[[:space:]]+" " " no-nl))
-         (limit gdb-locals-value-limit))
-    (if (>= (length str) limit)
-        (concat (substring str 0 limit) "...")
-      str)))
+  (let* ((no-nl (replace-regexp-in-string "\n" " " (or value "<Unknown>")))
+         (str (replace-regexp-in-string "[[:space:]]+" " " no-nl)))
+    str))
 
 (defun gdb-edit-locals-value (&optional event)
   "Assign a value to a variable displayed in the locals buffer."
@@ -4403,6 +4418,22 @@ gdb-edit-locals-value
       (gud-basic-call
        (concat  "-gdb-set variable " var " = " value)))))
 
+
+(defun gdb-locals-table-columns-list (alist)
+  "Format and arrange the columns in locals display based on ALIST."
+  (let (columns)
+    (dolist (config gdb-locals-table-row-config columns)
+      (let* ((key  (car config))
+             (max  (cdr config))
+             (prop (alist-get key alist)))
+        (when prop
+          (if (and (> max 0) (length> prop max))
+              (push (propertize (string-truncate-left prop max) 'help-echo prop)
+                    columns)
+            (push prop columns)))))
+    (nreverse columns)))
+
+
 ;; Complex data types are looked up in `gdb-locals-values-table'.
 (defun gdb-locals-handler-custom ()
   "Handler to rebuild the local variables table buffer."
@@ -4431,12 +4462,14 @@ gdb-locals-handler-custom
                                             help-echo "mouse-2: edit value"
                                             local-map ,gdb-edit-locals-map-1)
                                value))
+        (setf (gdb-table-right-align table) t)
+        (setq name (propertize name 'font-lock-face font-lock-variable-name-face))
+        (setq type (propertize type 'font-lock-face font-lock-type-face))
         (gdb-table-add-row
          table
-         (list
-          (propertize type 'font-lock-face font-lock-type-face)
-          (propertize name 'font-lock-face font-lock-variable-name-face)
-          value)
+         (gdb-locals-table-columns-list `((name  . ,name)
+                                          (type  . ,type)
+                                          (value . ,value)))
          `(gdb-local-variable ,local))))
     (insert (gdb-table-string table " "))
     (setq mode-name
-- 
2.34.1

bug#59730: gdb-mi.el: Local variables reordering

[Eli Zaretskii]

> From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> Date: Sun, 4 Dec 2022 15:26:22 +0100
> Cc: 59730@debbugs.gnu.org
> 
> There I also noticed that that the `gdb-locals-value-limit'
> has become redundant with this change. Unfortunately, I wasn't able to figure out how
> to mark that defcustom as deprecated, is there some kind of macro that should be done
> for that, or should I just remove it?

I don't like making backwards-incompatible changes such as removing a
user option.  How about if the default value of
gdb-locals-table-row-config used the value of gdb-locals-value-limit
instead of a literal 100?

> +** Variable order and truncation can now be configured in gdb-many-window mode.
> +The new variable `gdb-locals-table-row-config' allows users to
> +configure the order and max length of various properties in the local
> +variables frame.
             ^^^^^
"frame or window", right?

> +By default, this variable is set to write the properties in the order:
> +name, type and value. Where name and type are truncated to 20
                       ^^
Our conventions are to use two spaces between sentences.

> +characters, and the value is truncated to 100.
> +
> +In order to restore the old display behavior, set
> +`gdb-locals-table-row-config' to '((type . 0)(name . 0)(value . 0)).

That value is incorrect, because the value was truncated at 100, due
to gdb-locals-value-limit, right?

> +Additionally, this behavior encompasses the actions of the old
> +`gdb-locals-value-limit' variable, which is no longer used and marked
> +deprecated.

I hope we won't need this paragraph.

Thanks.

bug#59730: gdb-mi.el: Local variables reordering

[Gustaf Waldemarson]

[-- Attachment #1.1: Type: text/plain, Size: 2832 bytes --]

> I don't like making backwards-incompatible changes such as removing a
> user option.  How about if the default value of
> gdb-locals-table-row-config used the value of gdb-locals-value-limit
> instead of a literal 100?

Done

>  "frame or window", right?

I had to do a double take on this to make sure I was using the right
terminology,
at least according to the Emacs GDB documentation
(
https://www.gnu.org/software/emacs/manual/html_node/emacs/GDB-User-Interface-Layout.html
)
it seems like it should be the "local variables buffer", so I changed the
news entry
to match that, hope that's alright.

>  Our conventions are to use two spaces between sentences.

Should be fixed now as well

> That value is incorrect, because the value was truncated at 100, due
> to gdb-locals-value-limit, right?

Indeed, that was my bad and should be fixed now

>  I hope we won't need this paragraph.

Indeed, it is removed as requested

That should be everything I think, if there's anything else, please just
say so,
and thanks again for the feedback!

Best regards,
Gustaf

Den ons 7 dec. 2022 kl 18:50 skrev Eli Zaretskii <eliz@gnu.org>:

> > From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> > Date: Sun, 4 Dec 2022 15:26:22 +0100
> > Cc: 59730@debbugs.gnu.org
> >
> > There I also noticed that that the `gdb-locals-value-limit'
> > has become redundant with this change. Unfortunately, I wasn't able to
> figure out how
> > to mark that defcustom as deprecated, is there some kind of macro that
> should be done
> > for that, or should I just remove it?
>
> I don't like making backwards-incompatible changes such as removing a
> user option.  How about if the default value of
> gdb-locals-table-row-config used the value of gdb-locals-value-limit
> instead of a literal 100?
>
> > +** Variable order and truncation can now be configured in
> gdb-many-window mode.
> > +The new variable `gdb-locals-table-row-config' allows users to
> > +configure the order and max length of various properties in the local
> > +variables frame.
>              ^^^^^
> "frame or window", right?
>
> > +By default, this variable is set to write the properties in the order:
> > +name, type and value. Where name and type are truncated to 20
>                        ^^
> Our conventions are to use two spaces between sentences.
>
> > +characters, and the value is truncated to 100.
> > +
> > +In order to restore the old display behavior, set
> > +`gdb-locals-table-row-config' to '((type . 0)(name . 0)(value . 0)).
>
> That value is incorrect, because the value was truncated at 100, due
> to gdb-locals-value-limit, right?
>
> > +Additionally, this behavior encompasses the actions of the old
> > +`gdb-locals-value-limit' variable, which is no longer used and marked
> > +deprecated.
>
> I hope we won't need this paragraph.
>
> Thanks.
>

[-- Attachment #1.2: Type: text/html, Size: 4045 bytes --]

[-- Attachment #2: 0001-gdb-mi.el-Configure-variable-order-and-length.patch --]
[-- Type: application/x-patch, Size: 5069 bytes --]

bug#59730: gdb-mi.el: Local variables reordering

[Eli Zaretskii]

> From: Gustaf Waldemarson <gustaf.waldemarson@gmail.com>
> Date: Thu, 8 Dec 2022 19:43:46 +0100
> Cc: 59730@debbugs.gnu.org
> 
> > I don't like making backwards-incompatible changes such as removing a
> > user option.  How about if the default value of
> > gdb-locals-table-row-config used the value of gdb-locals-value-limit
> > instead of a literal 100?
> 
> Done
> 
> >  "frame or window", right?
> 
> I had to do a double take on this to make sure I was using the right terminology,
> at least according to the Emacs GDB documentation
> (https://www.gnu.org/software/emacs/manual/html_node/emacs/GDB-User-Interface-Layout.html)
> it seems like it should be the "local variables buffer", so I changed the news entry
> to match that, hope that's alright.
> 
> >  Our conventions are to use two spaces between sentences.
> 
> Should be fixed now as well
> 
> > That value is incorrect, because the value was truncated at 100, due
> > to gdb-locals-value-limit, right?
> 
> Indeed, that was my bad and should be fixed now
> 
> >  I hope we won't need this paragraph.
> 
> Indeed, it is removed as requested
> 
> That should be everything I think, if there's anything else, please just say so,
> and thanks again for the feedback!

Thanks, I installed the changes on the master branch.

In the future please accompany the changes with a ChangeLog-style
description of actual changes in the changeset; see CONTRIBUTE for the
details and "git log" for examples.