bug#58547: 29.0.50; [PATCH] Fix one internal naming

bug#58547: 29.0.50; [PATCH] Fix one internal naming

[Manuel Giraud]

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


Hi,

I don't know if those kind of bikesheeding patch are accepted but I
think that "estimate_mode_line_height" is not a good name since it can
be (and is!) called with others face id.

Best regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-one-internal-naming.patch --]
[-- Type: text/x-patch, Size: 4425 bytes --]

From e7c3a8f3d48ca03e521bd59bf24b795d5853b70d Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sat, 15 Oct 2022 12:56:50 +0200
Subject: [PATCH] Fix one internal naming

* src/xdisp.c (estimate_line_height_with_face): Renaming of
estimate_mode_line_height.

* src/dispextern.h: Update calls.
---
 src/dispextern.h |  8 ++++----
 src/xdisp.c      | 14 +++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index 2f5f4335fe..c673b7e30b 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1514,7 +1514,7 @@ #define CURRENT_MODE_LINE_HEIGHT(W)					\
    : ((W)->mode_line_height						\
       = (MATRIX_MODE_LINE_HEIGHT ((W)->current_matrix)			\
 	 ? MATRIX_MODE_LINE_HEIGHT ((W)->current_matrix)		\
-	 : estimate_mode_line_height					\
+	 : estimate_line_height_with_face				\
 	 (XFRAME ((W)->frame), CURRENT_MODE_LINE_ACTIVE_FACE_ID (W)))))
 
 /* Return the current height of the header line of window W.  If not known
@@ -1527,7 +1527,7 @@ #define CURRENT_HEADER_LINE_HEIGHT(W)				\
    : ((W)->header_line_height					\
       = (MATRIX_HEADER_LINE_HEIGHT ((W)->current_matrix)	\
 	 ? MATRIX_HEADER_LINE_HEIGHT ((W)->current_matrix)	\
-	 : estimate_mode_line_height				\
+	 : estimate_line_height_with_face			\
 	     (XFRAME ((W)->frame), HEADER_LINE_FACE_ID))))
 
 /* Return the current height of the tab line of window W.  If not known
@@ -1540,7 +1540,7 @@ #define CURRENT_TAB_LINE_HEIGHT(W)				\
    : ((W)->tab_line_height					\
       = (MATRIX_TAB_LINE_HEIGHT ((W)->current_matrix)		\
 	 ? MATRIX_TAB_LINE_HEIGHT ((W)->current_matrix)		\
-	 : estimate_mode_line_height				\
+	 : estimate_line_height_with_face			\
 	     (XFRAME ((W)->frame), TAB_LINE_FACE_ID))))
 
 /* Return the height of the desired mode line of window W.  */
@@ -3392,7 +3392,7 @@ #define TTY_CAP_STRIKE_THROUGH	0x20
 int window_box_left (struct window *, enum glyph_row_area);
 int window_box_left_offset (struct window *, enum glyph_row_area);
 int window_box_right (struct window *, enum glyph_row_area);
-int estimate_mode_line_height (struct frame *, enum face_id);
+int estimate_line_height_with_face (struct frame *, enum face_id);
 int move_it_to (struct it *, ptrdiff_t, int, int, int, int);
 void pixel_to_glyph_coords (struct frame *, int, int, int *, int *,
                             NativeRectangle *, bool);
diff --git a/src/xdisp.c b/src/xdisp.c
index e390de6a33..6944f13c47 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -1274,7 +1274,7 @@ window_box_height (struct window *w)
      height is essentially the same as that contained in the macro
      CURRENT_{MODE,HEADER,TAB}_LINE_HEIGHT, except that it checks whether
      the appropriate glyph row has its `mode_line_p' flag set, and if
-     it doesn't, uses estimate_mode_line_height instead.  */
+     it doesn't, uses estimate_line_height_with_face instead.  */
 
   if (window_wants_mode_line (w))
     {
@@ -1289,7 +1289,7 @@ window_box_height (struct window *w)
 	  if (ml_row && ml_row->mode_line_p)
 	    height -= ml_row->height;
 	  else
-	    height -= estimate_mode_line_height
+	    height -= estimate_line_height_with_face
 	      (f, CURRENT_MODE_LINE_ACTIVE_FACE_ID (w));
 	}
     }
@@ -1307,7 +1307,7 @@ window_box_height (struct window *w)
 	  if (tl_row && tl_row->mode_line_p)
 	    height -= tl_row->height;
 	  else
-	    height -= estimate_mode_line_height (f, TAB_LINE_FACE_ID);
+	    height -= estimate_line_height_with_face (f, TAB_LINE_FACE_ID);
 	}
     }
 
@@ -1324,7 +1324,7 @@ window_box_height (struct window *w)
 	  if (hl_row && hl_row->mode_line_p)
 	    height -= hl_row->height;
 	  else
-	    height -= estimate_mode_line_height (f, HEADER_LINE_FACE_ID);
+	    height -= estimate_line_height_with_face (f, HEADER_LINE_FACE_ID);
 	}
     }
 
@@ -2241,11 +2241,11 @@ compute_string_pos (struct text_pos *newpos, struct text_pos pos, Lisp_Object st
 }
 
 /* EXPORT:
-   Return an estimation of the pixel height of mode or header lines on
-   frame F.  FACE_ID specifies what line's height to estimate.  */
+   Return an estimation of the pixel height of lines on frame F.
+   FACE_ID specifies what line's height to estimate.  */
 
 int
-estimate_mode_line_height (struct frame *f, enum face_id face_id)
+estimate_line_height_with_face (struct frame *f, enum face_id face_id)
 {
 #ifdef HAVE_WINDOW_SYSTEM
   if (FRAME_WINDOW_P (f))
-- 
2.38.0


[-- Attachment #3: Type: text/plain, Size: 9129 bytes --]



In GNU Emacs 29.0.50 (build 1, x86_64-unknown-openbsd7.2, cairo version
 1.17.6) of 2022-10-14 built on elite.giraud
Repository revision: 100ed84198cd97664a94e89d1730e97b1fabbbb9
Repository branch: mgi/play
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: OpenBSD elite.giraud 7.2 GENERIC.MP#778 amd64

Configured using:
 'configure --prefix=/home/manuel/emacs --bindir=/home/manuel/bin
 --with-x-toolkit=no --without-sound --without-compress-install
 CPPFLAGS=-I/usr/local/include LDFLAGS=-L/usr/local/lib'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBXML2 MODULES NOTIFY KQUEUE OLDXMENU PDUMPER PNG RSVG
SQLITE3 THREADS TIFF WEBP X11 XDBE XIM XINPUT2 XPM ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  gnus-topic-mode: t
  icomplete-mode: t
  display-time-mode: t
  display-battery-mode: t
  gnus-undo-mode: t
  shell-dirtrack-mode: t
  global-so-long-mode: t
  repeat-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tab-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/manuel/.el/exwm/exwm hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm
/home/manuel/.el/exwm/exwm-xim hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-xim
/home/manuel/.el/exwm/exwm-workspace hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-workspace
/home/manuel/.el/exwm/exwm-randr hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-randr
/home/manuel/.el/exwm/exwm-manage hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-manage
/home/manuel/.el/exwm/exwm-layout hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-layout
/home/manuel/.el/exwm/exwm-input hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-input
/home/manuel/.el/exwm/exwm-floating hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-floating
/home/manuel/.el/exwm/exwm-core hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-core
/home/manuel/.el/exwm/exwm-config hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-config
/home/manuel/.el/exwm/exwm-cm hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-cm
/home/manuel/.el/exwm/exwm-systemtray hides /home/manuel/.emacs.d/elpa/exwm-0.26/exwm-systemtray
/home/manuel/.emacs.d/elpa/transient-20220918.2101/transient hides /home/manuel/emacs/share/emacs/29.0.50/lisp/transient

Features:
(shadow emacsbug tmm proced gnus-dired magit-patch dabbrev magit-extras
magit-bookmark magit-submodule magit-obsolete magit-blame magit-stash
magit-reflog magit-bisect magit-push magit-pull magit-fetch magit-clone
magit-remote magit-commit magit-sequence magit-notes magit-worktree
magit-tag magit-merge magit-branch magit-reset magit-files magit-refs
magit-status magit magit-repos magit-apply magit-wip magit-log
which-func imenu magit-diff git-commit log-edit add-log magit-core
magit-autorevert autorevert magit-margin magit-transient magit-process
with-editor magit-mode transient magit-git magit-base magit-section dash
compat-27 compat-26 compat compat-macs find-dired ffap misearch
multi-isearch cus-start bug-reference whitespace vc-hg vc-git vc-bzr
vc-src vc-sccs vc-cvs vc-rcs log-view pcvs-util vc-dir ewoc vc mailalias
smerge-mode diff diff-mode sort gnus-cite mail-extr textsec uni-scripts
idna-mapping ucs-normalize uni-confusable textsec-check gnus-async
gnus-bcklg gnus-ml gnus-topic mm-archive url-http url-gw url-cache
url-auth qp utf-7 imap rfc2104 nndoc nndraft nnmh network-stream nsm
nnfolder nnml gnus-agent gnus-srvr gnus-score score-mode nnvirtual nntp
gnus-cache nnrss w3m w3m-hist w3m-fb bookmark-w3m w3m-ems w3m-favicon
w3m-image tab-line w3m-proc w3m-util cal-china lunar solar cal-dst
cal-bahai cal-islam cal-hebrew holidays holiday-loaddefs cal-iso
org-indent face-remap org-agenda org-refile pulse sh-script smie
executable vc-dispatcher vc-svn view org-element avl-tree ol-eww eww xdg
url-queue mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect
ol-docview doc-view filenotify jka-compr image-mode exif ol-bibtex
ol-bbdb ol-w3m ol-doi org-link-doi epa-file paredit edmacro icomplete
time battery exwm-randr xcb-randr exwm-config exwm exwm-input
xcb-keysyms xcb-xkb exwm-manage exwm-floating xcb-cursor xcb-render
exwm-layout exwm-workspace exwm-core xcb-ewmh xcb-icccm xcb xcb-xproto
xcb-types xcb-debug kmacro server stimmung-themes modus-operandi-theme
modus-themes ytdious osm mingus libmpdee reporter edebug debug backtrace
transmission diary-lib diary-loaddefs color calc-bin calc-ext calc
calc-loaddefs rect calc-macs w3m-load mu4e mu4e-org mu4e-main mu4e-view
mu4e-headers mu4e-compose mu4e-draft mu4e-actions smtpmail mu4e-search
mu4e-lists mu4e-bookmarks mu4e-mark mu4e-message flow-fill mule-util
hl-line mu4e-contacts mu4e-update mu4e-folders mu4e-server mu4e-context
mu4e-vars mu4e-helpers mu4e-config bookmark ido supercite regi
ebdb-message ebdb-gnus gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime
smime gnutls dig gnus-sum shr pixel-fill kinsoku url-file svg dom
gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud nnimap nnmail
mail-source utf7 nnoo gnus-spec gnus-int gnus-range message sendmail
yank-media puny rfc822 mml mml-sec epa epg rfc6068 epg-config mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums
gmm-utils mailheader gnus-win gnus nnheader gnus-util mail-utils range
mm-util mail-prsvr ebdb-mua ebdb-com crm ebdb-format ebdb mailabbrev
eieio-opt speedbar ezimage dframe eieio-base pcase timezone org ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src
ob-comint org-pcomplete org-list org-faces org-entities org-version
ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex ol org-keys oc
org-compat org-macs org-loaddefs find-func cal-menu calendar
cal-loaddefs visual-basic-mode cl web-mode disp-table erlang-start
smart-tabs-mode skeleton cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs slime-asdf grep slime-tramp
tramp tramp-loaddefs trampver tramp-integration cus-edit cus-load
wid-edit files-x tramp-compat rx shell pcomplete parse-time iso8601
time-date ls-lisp format-spec slime-fancy slime-indentation
slime-cl-indent cl-indent slime-trace-dialog slime-fontifying-fu
slime-package-fu slime-references slime-compiler-notes-tree
slime-scratch slime-presentations advice bridge slime-macrostep
macrostep slime-mdot-fu slime-enclosing-context slime-fuzzy
slime-fancy-trace slime-fancy-inspector slime-c-p-c
slime-editing-commands slime-autodoc slime-repl elp slime-parse slime
derived cl-extra help-mode lisp-mnt gud apropos compile
text-property-search etags fileloop generator xref project arc-mode
archive-mode noutline outline icons pp comint ansi-osc ansi-color ring
hyperspec thingatpt slime-autoloads dired-aux dired-x dired
dired-loaddefs so-long notifications dbus xml repeat easy-mmode
auctex-autoloads tex-site boxquote-autoloads debbugs-autoloads
hyperbole-autoloads magit-autoloads git-commit-autoloads
magit-section-autoloads dash-autoloads paredit-autoloads
rust-mode-autoloads stimmung-themes-autoloads transient-autoloads
with-editor-autoloads info compat-autoloads ytdious-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 cconv url-vars
cl-loaddefs cl-lib rmc iso-transl tooltip 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 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
kqueue lcms2 dynamic-setting system-font-setting font-render-setting
cairo xinput2 x multi-tty make-network-process emacs)

Memory information:
((conses 16 1254861 401274)
 (symbols 48 66088 14)
 (strings 32 304472 26174)
 (string-bytes 1 9736534)
 (vectors 16 187355)
 (vector-slots 8 3050822 121164)
 (floats 8 1139 746)
 (intervals 56 39518 847)
 (buffers 1000 63))

-- 
Manuel Giraud

bug#58547: 29.0.50; [PATCH] Fix one internal naming

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Date: Sat, 15 Oct 2022 14:55:19 +0200
> 
> I don't know if those kind of bikesheeding patch are accepted but I
> think that "estimate_mode_line_height" is not a good name since it can
> be (and is!) called with others face id.

These lines are all considered "mode lines" in the display code, and
have the mode_line_p flag set.  So "mode line" is used here in a
slightly different sense than it might seem at first glance.

That said, I would not object to renaming this function, but only if
the new name makes more sense.  "Line with face" is too general to
make sense, because basically every line on a typical Emacs display
has some face.

Also, if we are inventing a new collective name to these 3 display
lines, we need to rename the mode_line_p flag to have the same name.

Thanks.

bug#58547: 29.0.50; [PATCH] Fix one internal naming

[Manuel Giraud]

Hi Eli,

Yes, sorry I was too fast with this renaming I've just seen in a "struct
glyph_row" comment that mode_line is a synonym for tab_line and
header_line too.  Forget about it, I think that "mode_line" is ok and
this name is present in many places.
-- 
Manuel Giraud

bug#58547: 29.0.50; [PATCH] Fix one internal naming

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 58547@debbugs.gnu.org
> Date: Sat, 15 Oct 2022 15:37:47 +0200
> 
> Hi Eli,
> 
> Yes, sorry I was too fast with this renaming I've just seen in a "struct
> glyph_row" comment that mode_line is a synonym for tab_line and
> header_line too.  Forget about it, I think that "mode_line" is ok and
> this name is present in many places.

Right.  But still: I'm not against renaming them, if we find a better,
more general name.

bug#58547: 29.0.50; [PATCH] Fix one internal naming

[Manuel Giraud]

Eli Zaretskii <eliz@gnu.org> writes:

> Right.  But still: I'm not against renaming them, if we find a better,
> more general name.

Why not.  In fact (and FTR), I wanted to rename this function because it
is strangely similar to the function `x_menu_bar_height' that I have
introduced in bug#58515.  So maybe, one day, the menu_line could become
one those "mode lines", WDYT?
-- 
Manuel Giraud

bug#58547: 29.0.50; [PATCH] Fix one internal naming

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 58547@debbugs.gnu.org
> Date: Sat, 15 Oct 2022 16:02:00 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Right.  But still: I'm not against renaming them, if we find a better,
> > more general name.
> 
> Why not.  In fact (and FTR), I wanted to rename this function because it
> is strangely similar to the function `x_menu_bar_height' that I have
> introduced in bug#58515.  So maybe, one day, the menu_line could become
> one those "mode lines", WDYT?

That's possible, yes.

The existing ones are similar in that they use
mode/header/tab-line-format, and that one supports certain special
constructs that are implemented in the subroutines called by
mode_line_height.  So if the menu-bar will come close to supporting
the same features, it would make sense to try to add it to the group,
indeed.

bug#58547: 29.0.50; [PATCH] Fix one internal naming

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: 58547@debbugs.gnu.org
>> Date: Sat, 15 Oct 2022 15:37:47 +0200
>>
>> Hi Eli,
>>
>> Yes, sorry I was too fast with this renaming I've just seen in a "struct
>> glyph_row" comment that mode_line is a synonym for tab_line and
>> header_line too.  Forget about it, I think that "mode_line" is ok and
>> this name is present in many places.
>
> Right.  But still: I'm not against renaming them, if we find a better,
> more general name.

If anyone finds such a name, please send a new patch; I'm closing this
bug report for now.