From 042f0d6a14cd99eb9d33cfccc6239534bc40e712 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Thu, 29 Jun 2023 10:47:26 +0300 Subject: [PATCH 1/8] ; Fix documentation of ':box' face attribute * lisp/faces.el (set-face-attribute): Update the documentation of WIDTH in the :box face attribute. (Bug#64344) --- lisp/faces.el | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/lisp/faces.el b/lisp/faces.el index 8bf7e4429d9..696634b4ef7 100644 --- a/lisp/faces.el +++ b/lisp/faces.el @@ -794,19 +794,25 @@ around them. If VALUE is nil, explicitly don't draw boxes. If VALUE is t, draw a box with lines of width 1 in the foreground color of the face. If VALUE is a string, the string must be a color name, and the box is drawn in that color with a line width of 1. Otherwise, -VALUE must be a property list of the form `(:line-width WIDTH -:color COLOR :style STYLE)'. If a keyword/value pair is missing from -the property list, a default value will be used for the value, as -specified below. WIDTH specifies the width of the lines to draw; it -defaults to 1. If WIDTH is negative, the absolute value is the width -of the lines, and draw top/bottom lines inside the characters area, -not around it. COLOR is the name of the color to draw in, default is -the background color of the face for 3D boxes and `flat-button', and -the foreground color of the face for other boxes. STYLE specifies -whether a 3D box should be draw. If STYLE is `released-button', draw -a box looking like a released 3D button. If STYLE is `pressed-button' -draw a box that appears like a pressed button. If STYLE is nil, -`flat-button' or omitted, draw a 2D box. +VALUE must be a property list of the following form: + + (:line-width WIDTH :color COLOR :style STYLE) + +If a keyword/value pair is missing from the property list, a default +value will be used for the value, as specified below. + +WIDTH specifies the width of the lines to draw; it defaults to 1. +If WIDTH is negative, the absolute value is the width of the lines, +and draw top/bottom lines inside the characters area, not around it. +WIDTH can also be a cons (VWIDTH . HWIDTH), which specifies different +values for the vertical and the horizontal line width. +COLOR is the name of the color to use for the box lines, default is +the background color of the face for 3D and `flat-button' boxes, and +the foreground color of the face for the other boxes. +STYLE specifies whether a 3D box should be drawn. If STYLE +is `released-button', draw a box looking like a released 3D button. +If STYLE is `pressed-button', draw a box that looks like a pressed +button. If STYLE is nil, `flat-button', or omitted, draw a 2D box. `:inverse-video' From cecbe92d5d99c427bcbeafc6ee2e53d6aac334e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= Date: Thu, 29 Jun 2023 11:21:53 +0200 Subject: [PATCH 2/8] ; * lisp/misc.el (duplicate-line-final-position): doc precision --- etc/NEWS | 2 +- lisp/misc.el | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index aa3b758a815..9e6f0c16bcd 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -698,7 +698,7 @@ between these modes while the user is inputting a command by hitting works like 'duplicate-line'. An active rectangular region is duplicated on its right-hand side. The new user option 'duplicate-line-final-position' specifies where to move point -after duplicating the line. +after duplicating a line. --- ** Files with the ".eld" extension are now visited in 'lisp-data-mode'. diff --git a/lisp/misc.el b/lisp/misc.el index 898fe9dd168..64f3986ff4c 100644 --- a/lisp/misc.el +++ b/lisp/misc.el @@ -64,7 +64,8 @@ Also see the `duplicate-line' command." (insert string))) (defcustom duplicate-line-final-position 0 - "Where to put point after duplicating the line with `duplicate-line'. + "Where to put point after duplicating a line with `duplicate-line' +or `duplicate-dwim'. When 0, leave point on the original line. When 1, move point to the first new line. When -1, move point to the last new line. From ddbb11f56572025d90497291de1dcaf2ece45500 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Thu, 29 Jun 2023 13:00:21 +0300 Subject: [PATCH 3/8] ; * lisp/misc.el (duplicate-line-final-position): Fix last doc change. Repeat after me: "The first line of a doc string must be a single complete sentence." --- lisp/misc.el | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lisp/misc.el b/lisp/misc.el index 64f3986ff4c..52df33911f7 100644 --- a/lisp/misc.el +++ b/lisp/misc.el @@ -64,8 +64,7 @@ Also see the `duplicate-line' command." (insert string))) (defcustom duplicate-line-final-position 0 - "Where to put point after duplicating a line with `duplicate-line' -or `duplicate-dwim'. + "Where to put point after `duplicate-line' or `duplicate-dwim'. When 0, leave point on the original line. When 1, move point to the first new line. When -1, move point to the last new line. From e982192e93369265cca7827065e13bf1f71aad13 Mon Sep 17 00:00:00 2001 From: Yuan Fu Date: Wed, 28 Jun 2023 14:16:52 -0700 Subject: [PATCH 4/8] Call treesit_record_change in subst-char-in-region (bug#64329) * src/editfns.c (Fsubst_char_in_region): Call treesit_record_change in the else branch. --- src/editfns.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/editfns.c b/src/editfns.c index d02cce4aef3..0cbeefb3262 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -55,6 +55,11 @@ along with GNU Emacs. If not, see . */ #ifdef WINDOWSNT # include "w32common.h" #endif + +#ifdef HAVE_TREE_SITTER +#include "treesit.h" +#endif + static void update_buffer_properties (ptrdiff_t, ptrdiff_t); static Lisp_Object styled_format (ptrdiff_t, Lisp_Object *, bool); @@ -2391,6 +2396,14 @@ Both characters must have the same length of multi-byte form. */) if (NILP (noundo)) record_change (pos, 1); for (i = 0; i < len; i++) *p++ = tostr[i]; + +#ifdef HAVE_TREE_SITTER + /* In the previous branch, replace_range() notifies + changes to tree-sitter, but in this branch, we + modified buffer content manually, so we need to + notify tree-sitter manually. */ + treesit_record_change (pos_byte, pos_byte + len, pos_byte + len); +#endif } last_changed = pos + 1; } From 02b6be892fa1a30b42c3df21319dddd2f445175e Mon Sep 17 00:00:00 2001 From: Yuan Fu Date: Wed, 28 Jun 2023 17:03:19 -0700 Subject: [PATCH 5/8] Add missing calls to treesit_record_change in editfns.c These should be all that are missing. See the next commit for detail. * src/editfns.c (Ftranslate_region_internal): (Ftranspose_regions): Call treesit_record_change. --- src/editfns.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/editfns.c b/src/editfns.c index 0cbeefb3262..a1e48daf6c6 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -2603,6 +2603,15 @@ It returns the number of characters changed. */) *p++ = *str++; signal_after_change (pos, 1, 1); update_compositions (pos, pos + 1, CHECK_BORDER); + +#ifdef HAVE_TREE_SITTER + /* In the previous branch, replace_range() notifies + changes to tree-sitter, but in this branch, we + modified buffer content manually, so we need to + notify tree-sitter manually. */ + treesit_record_change (pos_byte, pos_byte + len, + pos_byte + len); +#endif } characters_changed++; } @@ -4776,6 +4785,13 @@ ring. */) adjust_markers_bytepos (start1, start1_byte, end2, end2_byte, 0); } +#ifdef HAVE_TREE_SITTER + /* I don't think it's common to transpose two far-apart regions, so + amalgamating the edit into one should be fine. This is what the + signal_after_change below does, too. */ + treesit_record_change (start1_byte, end2_byte, end2_byte); +#endif + signal_after_change (start1, end2 - start1, end2 - start1); return Qnil; } From 1d2ba6b363b2e41ca40c74f679f80363e04a54ed Mon Sep 17 00:00:00 2001 From: Yuan Fu Date: Wed, 28 Jun 2023 17:05:29 -0700 Subject: [PATCH 6/8] ; * admin/notes/tree-sitter/treesit_record_change: Update. --- admin/notes/tree-sitter/treesit_record_change | 180 +++++++++++++++++- 1 file changed, 174 insertions(+), 6 deletions(-) diff --git a/admin/notes/tree-sitter/treesit_record_change b/admin/notes/tree-sitter/treesit_record_change index 0dc6491e2d1..e80df4adfa7 100644 --- a/admin/notes/tree-sitter/treesit_record_change +++ b/admin/notes/tree-sitter/treesit_record_change @@ -3,10 +3,10 @@ NOTES ON TREESIT_RECORD_CHANGE It is vital that Emacs informs tree-sitter of every change made to the buffer, lest tree-sitter's parse tree would be corrupted/out of sync. -All buffer changes in Emacs are made through functions in insdel.c -(and casefiddle.c), I augmented functions in those files with calls to -treesit_record_change. Below is a manifest of all the relevant -functions in insdel.c as of Emacs 29: +Almost all buffer changes in Emacs are made through functions in +insdel.c (see below for exceptions), I augmented functions in insdel.c +with calls to treesit_record_change. Below is a manifest of all the +relevant functions in insdel.c as of Emacs 29: Function Calls ---------------------------------------------------------------------- @@ -43,8 +43,176 @@ insert_from_buffer but not insert_from_buffer_1. I also left a reminder comment. -As for casefiddle.c, do_casify_unibyte_region and +EXCEPTIONS + + +There are a couple of functions that replaces characters in-place +rather than insert/delete. They are in casefiddle.c and editfns.c. + +In casefiddle.c, do_casify_unibyte_region and do_casify_multibyte_region modifies buffer, but they are static functions and are called by casify_region, which calls treesit_record_change. Other higher-level functions calls -casify_region to do the work. \ No newline at end of file +casify_region to do the work. + +In editfns.c, subst-char-in-region and translate-region-internal might +replace characters in-place, I made them to call +treesit_record_change. transpose-regions uses memcpy to move text +around, it calls treesit_record_change too. + +I found these exceptions by grepping for signal_after_change and +checking each caller manually. Below is all the result as of Emacs 29 +and some comment for each one. Readers can use + +(highlight-regexp "^[^[:space:]]+?\\.c:[[:digit:]]+:[^z-a]+?$" 'highlight) + +to make things easier to read. + +grep [...] --color=auto -i --directories=skip -nH --null -e signal_after_change *.c + +callproc.c:789: calling prepare_to_modify_buffer and signal_after_change. +callproc.c:793: is one call to signal_after_change in each of the +callproc.c:800: signal_after_change hasn't. A continue statement +callproc.c:804: again, and this time signal_after_change gets called, + +Not code. + +callproc.c:820: signal_after_change (PT - nread, 0, nread); +callproc.c:863: signal_after_change (PT - process_coding.produced_char, + +Both are called in call-process. I don’t think we’ll ever use +tree-sitter in call-process’s stdio buffer, right? I didn’t check +line-by-line, but it seems to only use insert_1_both and del_range_2. + +casefiddle.c:558: signal_after_change (start, end - start - added, end - start); + +Called in casify-region, calls treesit_record_change. + +decompress.c:195: signal_after_change (data->orig, data->start - data->orig, + +Called in unwind_decompress, uses del_range_2, insdel function. + +decompress.c:334: signal_after_change (istart, iend - istart, unwind_data.nbytes); + +Called in zlib-decompress-region, uses del_range_2, insdel function. + +editfns.c:2139: signal_after_change (BEGV, size_a, ZV - BEGV); + +Called in replace-buffer-contents, which calls del_range and +Finsert_buffer_substring, both are ok. + +editfns.c:2416: signal_after_change (changed, + +Called in subst-char-in-region, which either calls replace_range (a +insdel function) or modifies buffer content by itself (need to call +treesit_record_change). + +editfns.c:2544: /* Reload as signal_after_change in last iteration may GC. */ + +Not code. + +editfns.c:2604: signal_after_change (pos, 1, 1); + +Called in translate-region-internal, which has three cases: + +if (nc != oc && nc >= 0) { + if (len != str_len) { + replace_range() + } else { + while (str_len-- > 0) + *p++ = *str++; + } +} +else if (nc < 0) { + replace_range() +} + +replace_range is ok, but in the case where it manually modifies buffer +content, it needs to call treesit_record_change. + +editfns.c:4779: signal_after_change (start1, end2 - start1, end2 - start1); + +Called in transpose-regions. It just uses memcpy’s and doesn’t use +insdel functions; needs to call treesit_record_change. + +fileio.c:4825: signal_after_change (PT, 0, inserted); + +Called in insert_file_contents. Uses insert_1_both (very first in the +function); del_range_1 and del_range_byte (the optimized way to +implement replace when decoding isn’t needed); del_range_byte and +insert_from_buffer (the optimized way used when decoding is needed); +decode_coding_gap or insert_from_gap_1 (I’m not sure the condition for +this, but anyway it’s safe). The function also calls memcpy and +memmove, but they are irrelevant: memcpy is used for decoding, and +memmove is moving stuff inside the gap for decode_coding_gap. + +I’d love someone to verify this function, since it’s so complicated +and large, but from what I can tell it’s safe. + +fns.c:3998: signal_after_change (XFIXNAT (beg), 0, inserted_chars); + +Called in base64-decode-region, uses insert_1_both and del_range_both, +safe. + +insdel.c:681: signal_after_change (opoint, 0, len); +insdel.c:696: signal_after_change (opoint, 0, len); +insdel.c:741: signal_after_change (opoint, 0, len); +insdel.c:757: signal_after_change (opoint, 0, len); +insdel.c:976: signal_after_change (opoint, 0, PT - opoint); +insdel.c:996: signal_after_change (opoint, 0, PT - opoint); +insdel.c:1187: signal_after_change (opoint, 0, PT - opoint); +insdel.c:1412: signal_after_change. */ +insdel.c:1585: signal_after_change (from, nchars_del, GPT - from); +insdel.c:1600: prepare_to_modify_buffer and never call signal_after_change. +insdel.c:1603: region once. Apart from signal_after_change, any caller of this +insdel.c:1747: signal_after_change (from, to - from, 0); +insdel.c:1789: signal_after_change (from, to - from, 0); +insdel.c:1833: signal_after_change (from, to - from, 0); +insdel.c:2223:signal_after_change (ptrdiff_t charpos, ptrdiff_t lendel, ptrdiff_t lenins) +insdel.c:2396: signal_after_change (begpos, endpos - begpos - change, endpos - begpos); + +I’ve checked all insdel functions. We can assume insdel functions are +all safe. + +json.c:790: signal_after_change (PT, 0, inserted); + +Called in json-insert, calls either decode_coding_gap or +insert_from_gap_1, both are safe. Calls memmove but it’s for +decode_coding_gap. + +keymap.c:2873: /* Insert calls signal_after_change which may GC. */ + +Not code. + +print.c:219: signal_after_change (PT - print_buffer.pos, 0, print_buffer.pos); + +Called in print_finish, calls copy_text and insert_1_both, safe. + +process.c:6365: process buffer is changed in the signal_after_change above. +search.c:2763: (see signal_before_change and signal_after_change). Try to error + +Not code. + +search.c:2777: signal_after_change (sub_start, sub_end - sub_start, SCHARS (newtext)); + +Called in replace_match. Calls replace_range, upcase-region, +upcase-initials-region (both calls casify_region in the end), safe. +Calls memcpy but it’s for string manipulation. + +textprop.c:1261: signal_after_change (XFIXNUM (start), XFIXNUM (end) - XFIXNUM (start), +textprop.c:1272: signal_after_change (XFIXNUM (start), XFIXNUM (end) - XFIXNUM (start), +textprop.c:1283: signal_after_change (XFIXNUM (start), XFIXNUM (end) - XFIXNUM (start), +textprop.c:1458: signal_after_change (XFIXNUM (start), XFIXNUM (end) - XFIXNUM (start), +textprop.c:1652: signal_after_change (XFIXNUM (start), XFIXNUM (end) - XFIXNUM (start), +textprop.c:1661: signal_after_change (XFIXNUM (start), XFIXNUM (end) - XFIXNUM (start), +textprop.c:1672: signal_after_change (XFIXNUM (start), XFIXNUM (end) - XFIXNUM (start), +textprop.c:1750: before changes are made and signal_after_change when we are done. +textprop.c:1752: and call signal_after_change before returning if MODIFIED. */ +textprop.c:1764: signal_after_change (XFIXNUM (start), +textprop.c:1778: signal_after_change (XFIXNUM (start), XFIXNUM (end) - XFIXNUM (start), +textprop.c:1791: signal_after_change (XFIXNUM (start), XFIXNUM (end) - XFIXNUM (start), +textprop.c:1810: signal_after_change (XFIXNUM (start), + +We don’t care about text property changes. + +Grep finished with 51 matches found at Wed Jun 28 15:12:23 From 9e8386bdacc890390bb90f69889117667019c979 Mon Sep 17 00:00:00 2001 From: john muhl Date: Wed, 28 Jun 2023 12:58:27 -0500 Subject: [PATCH 7/8] Support cons cell as value of ':line-width' box attribute * lisp/calculator.el (calculator-need-3-lines): Support values of a face's ':line-width' box attribute that are cons cells. (Bug#64344) Copyright-paperwork-exempt: yes --- lisp/calculator.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisp/calculator.el b/lisp/calculator.el index 6a1d960c3e4..dbfba0b5bb7 100644 --- a/lisp/calculator.el +++ b/lisp/calculator.el @@ -746,7 +746,7 @@ See the documentation for `calculator-mode' for more information." ;; use 3 lines (let* ((bx (face-attribute 'mode-line :box)) (lh (plist-get bx :line-width))) - (and bx (or (not lh) (> lh 0)))) + (and bx (or (not lh) (> (if (listp lh) (cdr lh) lh) 0)))) ;; if the mode line has an overline, use 3 lines (not (memq (face-attribute 'mode-line :overline) '(nil unspecified))))))) From 65f146cf1c275cfce2265a5911c6460374ef153b Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Thu, 29 Jun 2023 21:21:28 +0300 Subject: [PATCH 8/8] ; * lisp/calculator.el (calculator-need-3-lines): Fix last change. --- lisp/calculator.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisp/calculator.el b/lisp/calculator.el index dbfba0b5bb7..b744f11e1e9 100644 --- a/lisp/calculator.el +++ b/lisp/calculator.el @@ -746,7 +746,8 @@ See the documentation for `calculator-mode' for more information." ;; use 3 lines (let* ((bx (face-attribute 'mode-line :box)) (lh (plist-get bx :line-width))) - (and bx (or (not lh) (> (if (listp lh) (cdr lh) lh) 0)))) + ;; Value of `:line-width' can be either a number or a cons. + (and bx (or (not lh) (> (if (consp lh) (cdr lh) lh) 0)))) ;; if the mode line has an overline, use 3 lines (not (memq (face-attribute 'mode-line :overline) '(nil unspecified)))))))