* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
@ 2020-06-16 17:08 Simen Heggestøyl
0 siblings, 0 replies; 30+ messages in thread
From: Simen Heggestøyl @ 2020-06-16 17:08 UTC (permalink / raw)
To: 41897
In js-mode, the following JavaScript comment is filled as expected when
fill-paragraph (M-q) is used with point within it:
/*
* This is a long comment that should break over multiple lines when fill-paragraph is used.
*/
Is filled as:
/*
* This is a long comment that should break over multiple lines when
* fill-paragraph is used.
*/
In mhtml-mode however, the same JavaScript comment:
<html>
<script>
/*
* This is a long comment that should break over multiple lines when fill-paragraph is used.
*/
</script>
</html>
Is filled as:
<html>
<script>
/* * This is a long comment that should break over multiple lines
when fill-paragraph is used. */
</script>
</html>
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
[not found] ` <mailman.1991.1592327403.2541.bug-gnu-emacs@gnu.org>
@ 2020-06-20 17:18 ` Alan Mackenzie
2020-06-20 18:27 ` Simen Heggestøyl
[not found] ` <87d05ta8z9.fsf@simenheg@gmail.com>
0 siblings, 2 replies; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-20 17:18 UTC (permalink / raw)
To: Simen Heggestøyl; +Cc: acm, 41897
Hello, Simen.
In article <mailman.1991.1592327403.2541.bug-gnu-emacs@gnu.org> you wrote:
> In js-mode, the following JavaScript comment is filled as expected when
> fill-paragraph (M-q) is used with point within it:
> /*
> * This is a long comment that should break over multiple lines when fill-paragraph is used.
> */
> Is filled as:
> /*
> * This is a long comment that should break over multiple lines when
> * fill-paragraph is used.
> */
> In mhtml-mode however, the same JavaScript comment:
> <html>
> <script>
> /*
> * This is a long comment that should break over multiple lines when fill-paragraph is used.
> */
> </script>
> </html>
> Is filled as:
> <html>
> <script>
> /* * This is a long comment that should break over multiple lines
> when fill-paragraph is used. */
> </script>
> </html>
Yes. This is happening because the mechanism mhtml-mode is using to
"switch to" js-mode is imperfect. Having more than one major mode in a
buffer is a difficult problem for Emacs, and there are currently no
really satisfactory solutions.
"Switching to" another mode essentially means setting buffer local
variable copies. In the existing code, a variable essential to filling,
adaptive-fill-regexp, isn't getting set to a value suitable for js-mode.
Also, there was an error in the js-mode setting of the variable
comment-start-skip.
The patch below fixes both these errors, and seems to allow your test
case to work. However, it is an incomplete solution - in particular,
filling caused by typing at the end of a long line still isn't working
properly. Still, it may be better than nothing, at least for now. I
intend to carry on working on this bug.
You haven't said what version of Emacs you're running. The patch
applies cleanly both to the upcoming emacs-27 branch and the master
branch at GNU savannah. To apply it, cd into ..../emacs/lisp, and
execute:
$ patch -p2 < this-email
. Then byte-compile the two changed files and load them (possibly by
restarting Emacs).
Here's the patch:
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 04b449ecd2..f5b6a4c260 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -4570,7 +4570,7 @@ js-mode
;; Comments
(setq-local comment-start "// ")
- (setq-local comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
+ (setq-local comment-start-skip "\\(?://+\\|/\\*+\\)\\s *")
(setq-local comment-end "")
(setq-local fill-paragraph-function #'js-fill-paragraph)
(setq-local normal-auto-fill-function #'js-do-auto-fill)
diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
index 1ae07c0a30..069329f4e4 100644
--- a/lisp/textmodes/mhtml-mode.el
+++ b/lisp/textmodes/mhtml-mode.el
@@ -73,7 +73,8 @@ mhtml-tag-relative-indent
(defconst mhtml--crucial-variable-prefix
(regexp-opt '("comment-" "uncomment-" "electric-indent-"
- "smie-" "forward-sexp-function" "completion-" "major-mode"))
+ "smie-" "forward-sexp-function" "completion-" "major-mode"
+ "adaptive-fill-" "fill-"))
"Regexp matching the prefix of \"crucial\" buffer-locals we want to capture.")
(defconst mhtml--variable-prefix
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply related [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-20 17:18 ` bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode Alan Mackenzie
@ 2020-06-20 18:27 ` Simen Heggestøyl
[not found] ` <87d05ta8z9.fsf@simenheg@gmail.com>
1 sibling, 0 replies; 30+ messages in thread
From: Simen Heggestøyl @ 2020-06-20 18:27 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: 41897
[-- Attachment #1: Type: text/plain, Size: 775 bytes --]
Hi Alan, thanks for working on this.
Alan Mackenzie <acm@muc.de> writes:
> The patch below fixes both these errors, and seems to allow your test
> case to work.
Yes, this seems to fix it in both emacs-27 and master, thanks.
However in emacs-27 with the patch applied I now get a strange
side-effect: After filling the comment, mhtml-mode seems to lose track
of where the JavaScript portion of the buffer is. Please see the
attached image. The position where point is should still be recognized
as HTML+JS, but it's recognized as plain HTML+ instead after
filling. This doesn't happen on master.
> You haven't said what version of Emacs you're running.
I put the version number in the bug subject but forgot to mention it in
the description, sorry about that.
-- Simen
[-- Attachment #2: bug41897.png --]
[-- Type: image/png, Size: 47390 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
[not found] ` <87d05ta8z9.fsf@simenheg@gmail.com>
@ 2020-06-21 16:55 ` Alan Mackenzie
2020-06-22 19:17 ` Alan Mackenzie
1 sibling, 0 replies; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-21 16:55 UTC (permalink / raw)
To: Simen Heggestøyl; +Cc: 41897
Hello again, Simen.
On Sat, Jun 20, 2020 at 20:27:22 +0200, Simen Heggestøyl wrote:
> Alan Mackenzie <acm@muc.de> writes:
> > The patch below fixes both these errors, and seems to allow your test
> > case to work.
> Yes, this seems to fix it in both emacs-27 and master, thanks.
> However in emacs-27 with the patch applied I now get a strange
> side-effect: After filling the comment, mhtml-mode seems to lose track
> of where the JavaScript portion of the buffer is. Please see the
> attached image. The position where point is should still be recognized
> as HTML+JS, but it's recognized as plain HTML+ instead after
> filling. This doesn't happen on master.
Yes. mhtml-mode keeps track of the position of the JavaScript bit by
applying text-properties. In certain circumstances, when point is at
(point-min), it fails to re-apply those properties after removing them.
This was happening in the filling, which uses narrowing. I think I've
got this fixed, now. (See patch below.)
> > You haven't said what version of Emacs you're running.
> I put the version number in the bug subject but forgot to mention it in
> the description, sorry about that.
No, my apologies: I completely missed it in the subject line. I'm happy
that you're working in master, and not Emacs 26. :-)
Anyhow, I've made significant progress on the problem. As well as the
problem above, I've repaired a few defects with auto-fill-mode. It's
more or less working. But M-q isn't completely working. If a buffer
contains the long line of your test case, followed by another line, M-q
doesn't fill them together, leaving the short line untouched.
I've had problems with auto-fill-function, and the current state in the
patch involves mhtml-mode assuming auto-fill-function is enabled. This
obviously needs fixing, but that's the way it is for now. Sorry.
There's still a bug with M-q where it sometimes throws an error about a
search being "on the wrong side of point". I haven't had time to
investigate this, yet.
So here's the current patch for the problem. Please install it on
master after removing yesterday's patch. From the .../emacs/lisp
directory, it should work with $ patch -p2 < this-email. (Or, maybe
there's a git command to do it more directly.)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 04b449ecd2..f5b6a4c260 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -4570,7 +4570,7 @@ js-mode
;; Comments
(setq-local comment-start "// ")
- (setq-local comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
+ (setq-local comment-start-skip "\\(?://+\\|/\\*+\\)\\s *")
(setq-local comment-end "")
(setq-local fill-paragraph-function #'js-fill-paragraph)
(setq-local normal-auto-fill-function #'js-do-auto-fill)
diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
index 1ae07c0a30..c5970cbcd1 100644
--- a/lisp/textmodes/mhtml-mode.el
+++ b/lisp/textmodes/mhtml-mode.el
@@ -73,15 +73,18 @@ mhtml-tag-relative-indent
(defconst mhtml--crucial-variable-prefix
(regexp-opt '("comment-" "uncomment-" "electric-indent-"
- "smie-" "forward-sexp-function" "completion-" "major-mode"))
+ "smie-" "forward-sexp-function" "completion-" "major-mode"
+ "adaptive-fill-" "fill-" "auto-fill-function"))
"Regexp matching the prefix of \"crucial\" buffer-locals we want to capture.")
(defconst mhtml--variable-prefix
(regexp-opt '("font-lock-" "indent-line-function"))
"Regexp matching the prefix of buffer-locals we want to capture.")
-(defun mhtml--construct-submode (mode &rest args)
- "A wrapper for make-mhtml--submode that computes the buffer-local variables."
+(defun mhtml--construct-submode (mode crucial-localized localized &rest args)
+ "A wrapper for make-mhtml--submode that computes the buffer-local variables.
+CRUCIAL-LOCALIZED and LOCALIZED are lists of symbols which must
+be considered crucial-local and local variables respectively."
(let ((captured-locals nil)
(crucial-captured-locals nil)
(submode (apply #'make-mhtml--submode args)))
@@ -94,6 +97,16 @@ mhtml--construct-submode
(unless (variable-binding-locus 'font-lock-fontify-region-function)
(setq-local font-lock-fontify-region-function
#'font-lock-default-fontify-region))
+ ;; Get the values from global variables which might become buffer local.
+ (dolist (iter crucial-localized)
+ (let ((sym (if (symbolp iter) iter (car iter)))
+ (val (symbol-value (if (symbolp iter) iter (cdr iter)))))
+ (push (cons sym val) crucial-captured-locals)))
+ (dolist (iter localized)
+ (let ((sym (if (symbolp iter) iter (car iter)))
+ (val (symbol-value (if (symbolp iter) iter (cdr iter)))))
+ (push (cons sym val) captured-locals)))
+
(dolist (iter (buffer-local-variables))
(when (string-match mhtml--crucial-variable-prefix
(symbol-name (car iter)))
@@ -119,6 +132,8 @@ mhtml--mark-crucial-buffer-locals
(defconst mhtml--css-submode
(mhtml--construct-submode 'css-mode
+ '((auto-fill-function . normal-auto-fill-function))
+ nil
:name "CSS"
:end-tag "</style>"
:syntax-table css-mode-syntax-table
@@ -127,6 +142,8 @@ mhtml--css-submode
(defconst mhtml--js-submode
(mhtml--construct-submode 'js-mode
+ '((auto-fill-function . normal-auto-fill-function))
+ nil
:name "JS"
:end-tag "</script>"
:syntax-table js-mode-syntax-table
@@ -202,12 +219,16 @@ mhtml--stashed-crucial-variables
(defun mhtml--stash-crucial-variables ()
(setq mhtml--stashed-crucial-variables
(mapcar (lambda (sym)
- (cons sym (buffer-local-value sym (current-buffer))))
+ (if (boundp sym)
+ (cons sym (buffer-local-value sym (current-buffer)))
+ sym))
mhtml--crucial-variables)))
(defun mhtml--map-in-crucial-variables (alist)
(dolist (item alist)
- (set (car item) (cdr item))))
+ (if (symbolp item)
+ (makunbound item)
+ (set (car item) (cdr item)))))
(defun mhtml--pre-command ()
(let ((submode (get-text-property (point) 'mhtml-submode)))
@@ -255,17 +276,14 @@ mhtml--syntax-propertize
sgml-syntax-propertize-rules))
(defun mhtml-syntax-propertize (start end)
- ;; First remove our special settings from the affected text. They
- ;; will be re-applied as needed.
- (remove-list-of-text-properties start end
- '(syntax-table local-map mhtml-submode))
- (goto-char start)
- ;; Be sure to look back one character, because START won't yet have
- ;; been propertized.
- (unless (bobp)
- (let ((submode (get-text-property (1- (point)) 'mhtml-submode)))
- (if submode
- (mhtml--syntax-propertize-submode submode end))))
+ (let ((submode (get-text-property start 'mhtml-submode)))
+ ;; First remove our special settings from the affected text. They
+ ;; will be re-applied as needed.
+ (remove-list-of-text-properties start end
+ '(syntax-table local-map mhtml-submode))
+ (goto-char start)
+ (if submode
+ (mhtml--syntax-propertize-submode submode end)))
(sgml-syntax-propertize (point) end mhtml--syntax-propertize))
(defun mhtml-indent-line ()
> -- Simen
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply related [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
[not found] ` <87d05ta8z9.fsf@simenheg@gmail.com>
2020-06-21 16:55 ` Alan Mackenzie
@ 2020-06-22 19:17 ` Alan Mackenzie
2020-06-23 0:02 ` Dmitry Gutov
` (2 more replies)
1 sibling, 3 replies; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-22 19:17 UTC (permalink / raw)
To: Simen Heggestøyl; +Cc: acm, 41897
Hello, Simen.
We're gradually converging on working code. Since yesterday, the
improvements are:
(i) (slightly) better handling of auto-fill-mode, which is no longer
automatically enabled;
(ii) An enhancement to CC Mode, used by mhtml-mode and js-mode, which
invalidates a CC Mode cache used by the filling code. This gets rid of
a few strange looking bugs;
(iii) The inclusion of "paragraph-" in the "crucial variables" thing, so
that paragraph-start and paragraph-separate from js-mode get into
mhtml-mode.
On Sat, Jun 20, 2020 at 20:27:22 +0200, Simen Heggestøyl wrote:
> Hi Alan, thanks for working on this.
> Alan Mackenzie <acm@muc.de> writes:
> > The patch below fixes both these errors, and seems to allow your test
> > case to work.
> Yes, this seems to fix it in both emacs-27 and master, thanks.
> However in emacs-27 with the patch applied I now get a strange
> side-effect: After filling the comment, mhtml-mode seems to lose track
> of where the JavaScript portion of the buffer is. Please see the
> attached image. The position where point is should still be recognized
> as HTML+JS, but it's recognized as plain HTML+ instead after
> filling. This doesn't happen on master.
I think this is now properly fixed.
Would you please try the latest patch, which might, just might, be fully
working code. As always, this is a diff on the code _without_ previous
patches. I haven't tested bits of enclosed css-mode source - you're
certainly better equipped to do this than I am.
diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index 8c8296fd6d..d8279ba4f0 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -3209,6 +3209,24 @@ c-truncate-lit-pos-cache
c-semi-near-cache-limit (min c-semi-near-cache-limit pos)
c-full-near-cache-limit (min c-full-near-cache-limit pos)))
+(defun c-foreign-truncate-lit-pos-cache (beg _end)
+ "Truncate CC Mode's literal cache.
+
+This function should be added to the `before-change-functions'
+hook by major modes that use CC Mode's filling functionality
+without initializing CC Mode. Currently (2020-06) these are
+js-mode and mhtml-mode."
+ (c-truncate-lit-pos-cache beg))
+
+(defun c-foreign-init-lit-pos-cache ()
+ "Initialize CC Mode's literal cache.
+
+This function should be called from the mode functions of major
+modes which use CC Mode's filling functionality without
+initializing CC Mode. Currently (2020-06) these are js-mode and
+mhtml-mode."
+ (c-truncate-lit-pos-cache 1))
+
\f
;; A system for finding noteworthy parens before the point.
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 04b449ecd2..5c50e2accd 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -4570,7 +4570,7 @@ js-mode
;; Comments
(setq-local comment-start "// ")
- (setq-local comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
+ (setq-local comment-start-skip "\\(?://+\\|/\\*+\\)\\s *")
(setq-local comment-end "")
(setq-local fill-paragraph-function #'js-fill-paragraph)
(setq-local normal-auto-fill-function #'js-do-auto-fill)
@@ -4591,6 +4591,8 @@ js-mode
(setq imenu-create-index-function #'js--imenu-create-index)
;; for filling, pretend we're cc-mode
+ (c-foreign-init-lit-pos-cache)
+ (add-hook 'before-change-functions #'c-foreign-truncate-lit-pos-cache nil t)
(setq-local comment-line-break-function #'c-indent-new-comment-line)
(setq-local comment-multi-line t)
(setq-local electric-indent-chars
diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
index 1ae07c0a30..bed0ced618 100644
--- a/lisp/textmodes/mhtml-mode.el
+++ b/lisp/textmodes/mhtml-mode.el
@@ -73,7 +73,9 @@ mhtml-tag-relative-indent
(defconst mhtml--crucial-variable-prefix
(regexp-opt '("comment-" "uncomment-" "electric-indent-"
- "smie-" "forward-sexp-function" "completion-" "major-mode"))
+ "smie-" "forward-sexp-function" "completion-" "major-mode"
+ "adaptive-fill-" "fill-" "normal-auto-fill-function"
+ "paragraph-"))
"Regexp matching the prefix of \"crucial\" buffer-locals we want to capture.")
(defconst mhtml--variable-prefix
@@ -94,6 +96,7 @@ mhtml--construct-submode
(unless (variable-binding-locus 'font-lock-fontify-region-function)
(setq-local font-lock-fontify-region-function
#'font-lock-default-fontify-region))
+
(dolist (iter (buffer-local-variables))
(when (string-match mhtml--crucial-variable-prefix
(symbol-name (car iter)))
@@ -255,17 +258,14 @@ mhtml--syntax-propertize
sgml-syntax-propertize-rules))
(defun mhtml-syntax-propertize (start end)
- ;; First remove our special settings from the affected text. They
- ;; will be re-applied as needed.
- (remove-list-of-text-properties start end
- '(syntax-table local-map mhtml-submode))
- (goto-char start)
- ;; Be sure to look back one character, because START won't yet have
- ;; been propertized.
- (unless (bobp)
- (let ((submode (get-text-property (1- (point)) 'mhtml-submode)))
- (if submode
- (mhtml--syntax-propertize-submode submode end))))
+ (let ((submode (get-text-property start 'mhtml-submode)))
+ ;; First remove our special settings from the affected text. They
+ ;; will be re-applied as needed.
+ (remove-list-of-text-properties start end
+ '(syntax-table local-map mhtml-submode))
+ (goto-char start)
+ (if submode
+ (mhtml--syntax-propertize-submode submode end)))
(sgml-syntax-propertize (point) end mhtml--syntax-propertize))
(defun mhtml-indent-line ()
@@ -333,6 +333,18 @@ mhtml-mode
;: Hack
(js--update-quick-match-re)
+ ;; Setup the appropriate js-mode value of auto-fill-function.
+ (setf (mhtml--submode-crucial-captured-locals mhtml--js-submode)
+ (push (cons 'auto-fill-function
+ (if (and (boundp 'auto-fill-function) auto-fill-function)
+ #'js-do-auto-fill
+ nil))
+ (mhtml--submode-crucial-captured-locals mhtml--js-submode)))
+
+ ;; This mode might be using CC Mode's filling functionality.
+ (c-foreign-init-lit-pos-cache)
+ (add-hook 'before-change-functions #'c-foreign-truncate-lit-pos-cache nil t)
+
;; This is sort of a prog-mode as well as a text mode.
(run-hooks 'prog-mode-hook))
> -- Simen
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply related [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-22 19:17 ` Alan Mackenzie
@ 2020-06-23 0:02 ` Dmitry Gutov
2020-06-23 8:36 ` Alan Mackenzie
2020-06-25 20:49 ` Tom Tromey
2020-07-04 13:13 ` Alan Mackenzie
2 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-23 0:02 UTC (permalink / raw)
To: Alan Mackenzie, Simen Heggestøyl; +Cc: 41897
Hi Alan,
On 22.06.2020 22:17, Alan Mackenzie wrote:
> +(defun c-foreign-truncate-lit-pos-cache (beg _end)
> + "Truncate CC Mode's literal cache.
> +
> +This function should be added to the `before-change-functions'
> +hook by major modes that use CC Mode's filling functionality
> +without initializing CC Mode. Currently (2020-06) these are
> +js-mode and mhtml-mode."
> + (c-truncate-lit-pos-cache beg))
Could you explain this part?
Is that literal cache looked up once during filling, or is it used
multiple times during the execution of c-fill-paragraph? If the latter
(and it does serve as a cache this way), perhaps it could be cleared
once, at the beginning of c-fill-paragraph, instead of adding a runtime
cost to every edit?
Or if that's undesirable, js-fill-paragraph could do that.
This way, I think it would automatically make it compatible with
mmm-mode. Or at least more compatible.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-23 0:02 ` Dmitry Gutov
@ 2020-06-23 8:36 ` Alan Mackenzie
2020-06-23 14:23 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-23 8:36 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Simen Heggestøyl, acm, 41897
Hello, Dmitry.
On Tue, Jun 23, 2020 at 03:02:48 +0300, Dmitry Gutov wrote:
> Hi Alan,
> On 22.06.2020 22:17, Alan Mackenzie wrote:
> > +(defun c-foreign-truncate-lit-pos-cache (beg _end)
> > + "Truncate CC Mode's literal cache.
> > +
> > +This function should be added to the `before-change-functions'
> > +hook by major modes that use CC Mode's filling functionality
> > +without initializing CC Mode. Currently (2020-06) these are
> > +js-mode and mhtml-mode."
> > + (c-truncate-lit-pos-cache beg))
> Could you explain this part?
> Is that literal cache looked up once during filling, or is it used
> multiple times during the execution of c-fill-paragraph?
It's used several times during each filling operation. This cache
persists between commands, too.
> If the latter (and it does serve as a cache this way), perhaps it
> could be cleared once, at the beginning of c-fill-paragraph, instead
> of adding a runtime cost to every edit?
The cost is tiny. c-truncate-lit-pos-cache is a defsubst which does
nothing but three copies of
(setq cache-limit (min beg cache-limit))
. All the intricacies of manipulating the cache take place whilst it is
being used.
> Or if that's undesirable, js-fill-paragraph could do that.
No, it really has to be in a before-change-functions function, to keep
track of the bound of the valid cache.
> This way, I think it would automatically make it compatible with
> mmm-mode. Or at least more compatible.
Maybe putting the two c-foreign-* functions into mmm-mode would work.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-23 8:36 ` Alan Mackenzie
@ 2020-06-23 14:23 ` Dmitry Gutov
2020-06-23 16:28 ` Alan Mackenzie
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-23 14:23 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
On 23.06.2020 11:36, Alan Mackenzie wrote:
>> If the latter (and it does serve as a cache this way), perhaps it
>> could be cleared once, at the beginning of c-fill-paragraph, instead
>> of adding a runtime cost to every edit?
>
> The cost is tiny. c-truncate-lit-pos-cache is a defsubst which does
> nothing but three copies of
>
> (setq cache-limit (min beg cache-limit))
>
> . All the intricacies of manipulating the cache take place whilst it is
> being used.
>
>> Or if that's undesirable, js-fill-paragraph could do that.
>
> No, it really has to be in a before-change-functions function, to keep
> track of the bound of the valid cache.
So it's really fine if it's called from HTML/CSS hunks as well?
And there's no way to just "reset" it to an appropriate value?
>> This way, I think it would automatically make it compatible with
>> mmm-mode. Or at least more compatible.
>
> Maybe putting the two c-foreign-* functions into mmm-mode would work.
mmm-mode is a minor mode, it doesn't always deal with CC Mode.
And its configurations don't usually result in new major modes either.
I wouldn't say it's very hard to make it work, but I don't see a "neat"
way to do either.
Have you considered adding variables that hold the cache to
mhtml--crucial-variable-prefix as well? Would that make it work?
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-23 14:23 ` Dmitry Gutov
@ 2020-06-23 16:28 ` Alan Mackenzie
2020-06-23 17:59 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-23 16:28 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Simen Heggestøyl, 41897
Hello, Dmitry.
On Tue, Jun 23, 2020 at 17:23:53 +0300, Dmitry Gutov wrote:
> On 23.06.2020 11:36, Alan Mackenzie wrote:
> >> If the latter (and it does serve as a cache this way), perhaps it
> >> could be cleared once, at the beginning of c-fill-paragraph, instead
> >> of adding a runtime cost to every edit?
> > The cost is tiny. c-truncate-lit-pos-cache is a defsubst which does
> > nothing but three copies of
> > (setq cache-limit (min beg cache-limit))
> > . All the intricacies of manipulating the cache take place whilst it
> > is being used.
> >> Or if that's undesirable, js-fill-paragraph could do that.
> > No, it really has to be in a before-change-functions function, to
> > keep track of the bound of the valid cache.
> So it's really fine if it's called from HTML/CSS hunks as well?
Not only fine, but necessary. The literal cache contains entries that
record things like "C comment between positions 23 and 130". If somebody
inserts text before that comment, or inside of it, that cache entry is no
longer valid, and must be invalidated. Hence the necessity of the
before-change function.
> And there's no way to just "reset" it to an appropriate value?
No. Not without killing its utility as a cache.
> >> This way, I think it would automatically make it compatible with
> >> mmm-mode. Or at least more compatible.
> > Maybe putting the two c-foreign-* functions into mmm-mode would work.
> mmm-mode is a minor mode, it doesn't always deal with CC Mode.
The question to consider here is whether any sub-mode of mmm-mode uses CC
Mode's comment filling without initialising CC Mode. js-mode and
mhtml-mode do this.
> And its configurations don't usually result in new major modes either.
> I wouldn't say it's very hard to make it work, but I don't see a "neat"
> way to do either.
OK.
> Have you considered adding variables that hold the cache to
> mhtml--crucial-variable-prefix as well? Would that make it work?
Not without the before-change function, no. I'm trying to see what the
point of putting these variables into mhtml's crucial variables would be.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-23 16:28 ` Alan Mackenzie
@ 2020-06-23 17:59 ` Dmitry Gutov
2020-06-23 19:17 ` Alan Mackenzie
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-23 17:59 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
Hi Alan,
On 23.06.2020 19:28, Alan Mackenzie wrote:
>> So it's really fine if it's called from HTML/CSS hunks as well?
>
> Not only fine, but necessary. The literal cache contains entries that
> record things like "C comment between positions 23 and 130". If somebody
> inserts text before that comment, or inside of it, that cache entry is no
> longer valid, and must be invalidated. Hence the necessity of the
> before-change function.
But isn't CC Mode confused by chunks of text with a totally different
syntax?
>> And there's no way to just "reset" it to an appropriate value?
>
> No. Not without killing its utility as a cache.
What do you mean? Even if the cache is reset at the beginning of a
function, if the function refers to it multiple times, the first time
should refill the cache, and the rest of the calls will be able to make
use of it properly.
>>>> This way, I think it would automatically make it compatible with
>>>> mmm-mode. Or at least more compatible.
>
>>> Maybe putting the two c-foreign-* functions into mmm-mode would work.
>
>> mmm-mode is a minor mode, it doesn't always deal with CC Mode.
>
> The question to consider here is whether any sub-mode of mmm-mode uses CC
> Mode's comment filling without initialising CC Mode. js-mode and
> mhtml-mode do this.
js-mode can be one of its submodes. c-mode as well, but none of CC Mode
family of major modes ever worked okay with it, I think.
js-mode mostly works, aside from features like this one.
>> Have you considered adding variables that hold the cache to
>> mhtml--crucial-variable-prefix as well? Would that make it work?
>
> Not without the before-change function, no. I'm trying to see what the
> point of putting these variables into mhtml's crucial variables would be.
Hopefully, it would make the submode regions inside independent
"islands", so to speak. Each of them having its own cache structure
(used or not).
TBH I'm not sure if mhtml-mode does the save-and-restore dance which
would be necessary for this. mmm-mode does, though.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-23 17:59 ` Dmitry Gutov
@ 2020-06-23 19:17 ` Alan Mackenzie
2020-06-23 23:11 ` Dmitry Gutov
2020-06-25 20:53 ` Tom Tromey
0 siblings, 2 replies; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-23 19:17 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Simen Heggestøyl, acm, 41897
Hello, Dmitry.
On Tue, Jun 23, 2020 at 20:59:22 +0300, Dmitry Gutov wrote:
> Hi Alan,
> On 23.06.2020 19:28, Alan Mackenzie wrote:
> >> So it's really fine if it's called from HTML/CSS hunks as well?
> > Not only fine, but necessary. The literal cache contains entries
> > that record things like "C comment between positions 23 and 130". If
> > somebody inserts text before that comment, or inside of it, that
> > cache entry is no longer valid, and must be invalidated. Hence the
> > necessity of the before-change function.
> But isn't CC Mode confused by chunks of text with a totally different
> syntax?
It might well be, probably is. But this isn't CC Mode we're talking
about - just a tiny part of its low level functionality, namely the bit
dealing with literals and filling them.
> >> And there's no way to just "reset" it to an appropriate value?
> > No. Not without killing its utility as a cache.
> What do you mean? Even if the cache is reset at the beginning of a
> function, if the function refers to it multiple times, the first time
> should refill the cache, and the rest of the calls will be able to make
> use of it properly.
That's what I mean. The cache persists over commands, reducing the
amount of recalculation needed, particularly for fast typing. Refilling
it from scratch on every keypress would likely make it sluggish.
Anyhow, it works fine at the moment, so why change it?
[ .... ]
> >> mmm-mode is a minor mode, it doesn't always deal with CC Mode.
> > The question to consider here is whether any sub-mode of mmm-mode
> > uses CC Mode's comment filling without initialising CC Mode. js-mode
> > and mhtml-mode do this.
> js-mode can be one of its submodes. c-mode as well, but none of CC Mode
> family of major modes ever worked okay with it, I think.
Having several major modes in a single buffer has always been problematic
in Emacs. Personally, I think there needs to be amendments in the
low-level C code to support it properly, but I'm not able to do this work
on my own, and there doesn't seem to be enough enthusiasm on other
people's part to help out.
> js-mode mostly works, aside from features like this one.
With the current patch, comment filling should work fine in js-mode.
> >> Have you considered adding variables that hold the cache to
> >> mhtml--crucial-variable-prefix as well? Would that make it work?
> > Not without the before-change function, no. I'm trying to see what the
> > point of putting these variables into mhtml's crucial variables would be.
> Hopefully, it would make the submode regions inside independent
> "islands", so to speak. Each of them having its own cache structure
> (used or not).
Ah, OK. So, buffer positions would be offsets from the island start, or
something like that.
> TBH I'm not sure if mhtml-mode does the save-and-restore dance which
> would be necessary for this. mmm-mode does, though.
mhtml-mode does do saving and restoring of local variables. I can't
judge how well in comparison with, say, mmm-mode.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-23 19:17 ` Alan Mackenzie
@ 2020-06-23 23:11 ` Dmitry Gutov
2020-06-24 17:43 ` Alan Mackenzie
2020-06-25 20:53 ` Tom Tromey
1 sibling, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-23 23:11 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
On 23.06.2020 22:17, Alan Mackenzie wrote:
>> But isn't CC Mode confused by chunks of text with a totally different
>> syntax?
>
> It might well be, probably is. But this isn't CC Mode we're talking
> about - just a tiny part of its low level functionality, namely the bit
> dealing with literals and filling them.
So this cache is really serving the filling functionality only in this case.
>>>> And there's no way to just "reset" it to an appropriate value?
>
>>> No. Not without killing its utility as a cache.
>
>> What do you mean? Even if the cache is reset at the beginning of a
>> function, if the function refers to it multiple times, the first time
>> should refill the cache, and the rest of the calls will be able to make
>> use of it properly.
>
> That's what I mean. The cache persists over commands, reducing the
> amount of recalculation needed, particularly for fast typing. Refilling
> it from scratch on every keypress would likely make it sluggish.
Not on every keypress. Only when js-fill-paragraph is called. One-time
delay only when required.
> Anyhow, it works fine at the moment, so why change it?
The above scheme would require fewer references to CC Mode functions
from outside. js-mode support would automatically transfer to mhtml-mode
and mmm-mode with associated changes in them necessary.
One fewer before-change-functions element is also nothing to sneeze at.
>> js-mode can be one of its submodes. c-mode as well, but none of CC Mode
>> family of major modes ever worked okay with it, I think.
>
> Having several major modes in a single buffer has always been problematic
> in Emacs. Personally, I think there needs to be amendments in the
> low-level C code to support it properly, but I'm not able to do this work
> on my own, and there doesn't seem to be enough enthusiasm on other
> people's part to help out.
We've learned to deal with most other major modes and features in mmm
context.
>> js-mode mostly works, aside from features like this one.
>
> With the current patch, comment filling should work fine in js-mode.
Above, I meant that js-mode mostly works fine with mmm-mode. And my
suggestion might make comment filling work there, too. Automatically.
>>>> Have you considered adding variables that hold the cache to
>>>> mhtml--crucial-variable-prefix as well? Would that make it work?
>
>>> Not without the before-change function, no. I'm trying to see what the
>>> point of putting these variables into mhtml's crucial variables would be.
>
>> Hopefully, it would make the submode regions inside independent
>> "islands", so to speak. Each of them having its own cache structure
>> (used or not).
>
> Ah, OK. So, buffer positions would be offsets from the island start, or
> something like that.
Not necessarily, but possibly. The key aspect is that the cache inside a
particular submode is not affected by user actions outside of its
bounds. Not directly, at least.
But that's an mmm-mode feature anyway.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-23 23:11 ` Dmitry Gutov
@ 2020-06-24 17:43 ` Alan Mackenzie
2020-06-24 18:28 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-24 17:43 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Simen Heggestøyl, acm, 41897
Hello, Dmitry.
On Wed, Jun 24, 2020 at 02:11:31 +0300, Dmitry Gutov wrote:
> On 23.06.2020 22:17, Alan Mackenzie wrote:
[ .... ]
> >>>> And there's no way to just "reset" it to an appropriate value?
> >>> No. Not without killing its utility as a cache.
> >> What do you mean? Even if the cache is reset at the beginning of a
> >> function, if the function refers to it multiple times, the first time
> >> should refill the cache, and the rest of the calls will be able to make
> >> use of it properly.
> > That's what I mean. The cache persists over commands, reducing the
> > amount of recalculation needed, particularly for fast typing. Refilling
> > it from scratch on every keypress would likely make it sluggish.
> Not on every keypress. Only when js-fill-paragraph is called. One-time
> delay only when required.
But a substantial delay, involving (I think) analysing the code from BOB
each time. The current working setup has a negligible delay at each
buffer change (and, of course, recalculation of cache entries only when
required).
> > Anyhow, it works fine at the moment, so why change it?
> The above scheme would require fewer references to CC Mode functions
> from outside. js-mode support would automatically transfer to mhtml-mode
> and mmm-mode with associated changes in them necessary.
It sounds like you want to use a facility without initialising it. This
feels a bit unreasonable.
> One fewer before-change-functions element is also nothing to sneeze at.
There's nothing wrong with having functions in
before/after-change-functions. It's a standard Emacs programming
technique.
[ .... ]
> >> js-mode mostly works, aside from features like this one.
> > With the current patch, comment filling should work fine in js-mode.
> Above, I meant that js-mode mostly works fine with mmm-mode. And my
> suggestion might make comment filling work there, too. Automatically.
It works automatically at the moment (with the current patch applied). I
think you're saying again you don't want to be troubled by initialising
it.
> >>>> Have you considered adding variables that hold the cache to
> >>>> mhtml--crucial-variable-prefix as well? Would that make it work?
> >>> Not without the before-change function, no. I'm trying to see what the
> >>> point of putting these variables into mhtml's crucial variables would be.
> >> Hopefully, it would make the submode regions inside independent
> >> "islands", so to speak. Each of them having its own cache structure
> >> (used or not).
> > Ah, OK. So, buffer positions would be offsets from the island start, or
> > something like that.
> Not necessarily, but possibly. The key aspect is that the cache inside a
> particular submode is not affected by user actions outside of its
> bounds. Not directly, at least.
Well, when a cached holds buffer positions, something has to give -
either markers need to be used, or an offset from a variable position, or
something.
> But that's an mmm-mode feature anyway.
OK.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-24 17:43 ` Alan Mackenzie
@ 2020-06-24 18:28 ` Dmitry Gutov
2020-06-25 16:33 ` Alan Mackenzie
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-24 18:28 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
Hi Alan,
On 24.06.2020 20:43, Alan Mackenzie wrote:
>>> That's what I mean. The cache persists over commands, reducing the
>>> amount of recalculation needed, particularly for fast typing. Refilling
>>> it from scratch on every keypress would likely make it sluggish.
>
>> Not on every keypress. Only when js-fill-paragraph is called. One-time
>> delay only when required.
>
> But a substantial delay, involving (I think) analysing the code from BOB
> each time. The current working setup has a negligible delay at each
> buffer change (and, of course, recalculation of cache entries only when
> required).
I imagine that would not be a significant problem for the rare cases
that fill-paragraph is called in a JS region. Considering most of the
contents in mhtml-mode buffers are not JS code, on average, that should
tilt the scales in favor of parsing lazily, rather than affecting every
character insertion.
>>> Anyhow, it works fine at the moment, so why change it?
>
>> The above scheme would require fewer references to CC Mode functions
>> from outside. js-mode support would automatically transfer to mhtml-mode
>> and mmm-mode with associated changes in them necessary.
>
> It sounds like you want to use a facility without initialising it. This
> feels a bit unreasonable.
That cache reset at the beginning of js-fill-paragraph could as well
re-initialize the cache.
>> One fewer before-change-functions element is also nothing to sneeze at.
>
> There's nothing wrong with having functions in
> before/after-change-functions. It's a standard Emacs programming
> technique.
I'm not saying it's a terrible idea, but it has its downsides.
>>>> js-mode mostly works, aside from features like this one.
>
>>> With the current patch, comment filling should work fine in js-mode.
>
>> Above, I meant that js-mode mostly works fine with mmm-mode. And my
>> suggestion might make comment filling work there, too. Automatically.
>
> It works automatically at the moment (with the current patch applied). I
> think you're saying again you don't want to be troubled by initialising
> it.
It doesn't automatically work in mmm-mode. With my suggestion, it very
likely would.
>>>>>> Have you considered adding variables that hold the cache to
>>>>>> mhtml--crucial-variable-prefix as well? Would that make it work?
>
>>>>> Not without the before-change function, no. I'm trying to see what the
>>>>> point of putting these variables into mhtml's crucial variables would be.
>
>>>> Hopefully, it would make the submode regions inside independent
>>>> "islands", so to speak. Each of them having its own cache structure
>>>> (used or not).
>
>>> Ah, OK. So, buffer positions would be offsets from the island start, or
>>> something like that.
>
>> Not necessarily, but possibly. The key aspect is that the cache inside a
>> particular submode is not affected by user actions outside of its
>> bounds. Not directly, at least.
>
> Well, when a cached holds buffer positions, something has to give -
> either markers need to be used, or an offset from a variable position, or
> something.
Makes sense. The alternative would be to index the positions from the
beginning of the submode region. But then the related feature must
respect narrowing, and it would require explicit integration from
mmm-mode and friends.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-24 18:28 ` Dmitry Gutov
@ 2020-06-25 16:33 ` Alan Mackenzie
2020-06-25 16:48 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-25 16:33 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Simen Heggestøyl, acm, 41897
Hello, Dmitry.
On Wed, Jun 24, 2020 at 21:28:09 +0300, Dmitry Gutov wrote:
> Hi Alan,
> On 24.06.2020 20:43, Alan Mackenzie wrote:
[ .... ]
> > But a substantial delay, involving (I think) analysing the code from
> > BOB each time. The current working setup has a negligible delay at
> > each buffer change (and, of course, recalculation of cache entries
> > only when required).
> I imagine that would not be a significant problem for the rare cases
> that fill-paragraph is called in a JS region. Considering most of the
> contents in mhtml-mode buffers are not JS code, on average, that should
> tilt the scales in favor of parsing lazily, rather than affecting every
> character insertion.
The current patch does parse lazily. You want to remove the benefit of
using this cache, no matter how small, for reasons I still can't grasp.
This removal will hurt performance, and possibly cause new bugs to solve.
[ .... ]
> > It sounds like you want to use a facility without initialising it.
> > This feels a bit unreasonable.
> That cache reset at the beginning of js-fill-paragraph could as well
> re-initialize the cache.
You're misusing the work "initialize" here. If you initialise a variable
every time you read it, you might as well not have that variable.
[ .... ]
> >>>> js-mode mostly works, aside from features like this one.
> >>> With the current patch, comment filling should work fine in js-mode.
> >> Above, I meant that js-mode mostly works fine with mmm-mode. And my
> >> suggestion might make comment filling work there, too. Automatically.
> > It works automatically at the moment (with the current patch applied). I
> > think you're saying again you don't want to be troubled by initialising
> > it.
> It doesn't automatically work in mmm-mode. With my suggestion, it very
> likely would.
It would work fine with the current patch, together with calls to
initialise the mechanism. What precisely is the problem in mmm-mode?
[ .... ]
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 16:33 ` Alan Mackenzie
@ 2020-06-25 16:48 ` Dmitry Gutov
2020-06-25 18:07 ` Alan Mackenzie
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-25 16:48 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
On 25.06.2020 19:33, Alan Mackenzie wrote:
>> I imagine that would not be a significant problem for the rare cases
>> that fill-paragraph is called in a JS region. Considering most of the
>> contents in mhtml-mode buffers are not JS code, on average, that should
>> tilt the scales in favor of parsing lazily, rather than affecting every
>> character insertion.
>
> The current patch does parse lazily. You want to remove the benefit of
> using this cache, no matter how small, for reasons I still can't grasp.
That's not true. Like you confirmed, c-fill-paragraph refers to that
cache multiple times. We'll only make sure the cache is reset in the
beginning.
>>> It sounds like you want to use a facility without initialising it.
>>> This feels a bit unreasonable.
>
>> That cache reset at the beginning of js-fill-paragraph could as well
>> re-initialize the cache.
>
> You're misusing the work "initialize" here. If you initialise a variable
> every time you read it, you might as well not have that variable.
Like explained above, not *every* time.
>> It doesn't automatically work in mmm-mode. With my suggestion, it very
>> likely would.
>
> It would work fine with the current patch, together with calls to
> initialise the mechanism. What precisely is the problem in mmm-mode?
That there is no good place to plug in your new functions.
And, in general, to have per-mode before-change-functions contents.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 16:48 ` Dmitry Gutov
@ 2020-06-25 18:07 ` Alan Mackenzie
2020-06-25 18:19 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-25 18:07 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Simen Heggestøyl, acm, 41897
Hello, Dmitry.
On Thu, Jun 25, 2020 at 19:48:57 +0300, Dmitry Gutov wrote:
> On 25.06.2020 19:33, Alan Mackenzie wrote:
> >> I imagine that would not be a significant problem for the rare cases
> >> that fill-paragraph is called in a JS region. Considering most of the
> >> contents in mhtml-mode buffers are not JS code, on average, that should
> >> tilt the scales in favor of parsing lazily, rather than affecting every
> >> character insertion.
> > The current patch does parse lazily. You want to remove the benefit of
> > using this cache, no matter how small, for reasons I still can't grasp.
> That's not true. Like you confirmed, c-fill-paragraph refers to that
> cache multiple times. We'll only make sure the cache is reset in the
> beginning.
OK, first of all, I think I was mistaken in saying c-literal-limits is
called several times. I think it's just called once per filling action.
But if it were called several times, it would likely need to be updated
at every buffer change between calls.
The purpose of this cache is to avoid repeated scanning from BOB. Your
proposed continual splatting of it would remove the benefit of it
entirely.
> >>> It sounds like you want to use a facility without initialising it.
> >>> This feels a bit unreasonable.
> >> That cache reset at the beginning of js-fill-paragraph could as well
> >> re-initialize the cache.
> > You're misusing the work "initialize" here. If you initialise a
> > variable every time you read it, you might as well not have that
> > variable.
> Like explained above, not *every* time.
> >> It doesn't automatically work in mmm-mode. With my suggestion, it very
> >> likely would.
> > It would work fine with the current patch, together with calls to
> > initialise the mechanism. What precisely is the problem in mmm-mode?
> That there is no good place to plug in your new functions.
That would appear to be a deficiency in mmm-mode.
Does mmm-mode not call js-mode when that is one of the submodes? If it
doesn't, then why not add a general init function-variable/hook/whatever
into which initialisations can be plugged?
> And, in general, to have per-mode before-change-functions contents.
There's no problem with before/after-change-functions. They're the
canonical way to react to buffer changes.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 18:07 ` Alan Mackenzie
@ 2020-06-25 18:19 ` Dmitry Gutov
2020-06-25 19:13 ` Alan Mackenzie
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-25 18:19 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
Hi Alan,
On 25.06.2020 21:07, Alan Mackenzie wrote:
>> That's not true. Like you confirmed, c-fill-paragraph refers to that
>> cache multiple times. We'll only make sure the cache is reset in the
>> beginning.
>
> OK, first of all, I think I was mistaken in saying c-literal-limits is
> called several times. I think it's just called once per filling action.
>
> But if it were called several times, it would likely need to be updated
> at every buffer change between calls.
>
> The purpose of this cache is to avoid repeated scanning from BOB. Your
> proposed continual splatting of it would remove the benefit of it
> entirely.
That's unfortunate.
Guess the only thing that remains for me here is to express a wish for a
syntax-ppss based design here.
Because mmm-mode knows how to deal with major modes based on it, as a group.
>>> It would work fine with the current patch, together with calls to
>>> initialise the mechanism. What precisely is the problem in mmm-mode?
>
>> That there is no good place to plug in your new functions.
>
> That would appear to be a deficiency in mmm-mode.
>
> Does mmm-mode not call js-mode when that is one of the submodes? If it
> doesn't, then why not add a general init function-variable/hook/whatever
> into which initialisations can be plugged?
It does not pick up each and every hook.
If it did, though, it would only call your before-change-functions
inside js-mode regions, but it would have ignored them in HTML and CSS
regions. Which doesn't appear to be what you want anyway.
>> And, in general, to have per-mode before-change-functions contents.
>
> There's no problem with before/after-change-functions. They're the
> canonical way to react to buffer changes.
They're not very manageable, from mmm's point of view. And like the
current example shows, it's not obvious what to do with such hooks
outside of submode regions of major modes that added them.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 18:19 ` Dmitry Gutov
@ 2020-06-25 19:13 ` Alan Mackenzie
2020-06-25 19:28 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-25 19:13 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Simen Heggestøyl, acm, 41897
Hello, Dmitry.
On Thu, Jun 25, 2020 at 21:19:11 +0300, Dmitry Gutov wrote:
> Hi Alan,
> On 25.06.2020 21:07, Alan Mackenzie wrote:
> > The purpose of this cache is to avoid repeated scanning from BOB.
> > Your proposed continual splatting of it would remove the benefit of
> > it entirely.
> That's unfortunate.
Indeed. Let's assume that keeping it working is a requirement here.
> Guess the only thing that remains for me here is to express a wish for a
> syntax-ppss based design here.
> Because mmm-mode knows how to deal with major modes based on it, as a group.
How about enhancing mmm-mode to handle any major mode, rather than a
restricted subset?
> >>> It would work fine with the current patch, together with calls to
> >>> initialise the mechanism. What precisely is the problem in mmm-mode?
> >> That there is no good place to plug in your new functions.
> > That would appear to be a deficiency in mmm-mode.
> > Does mmm-mode not call js-mode when that is one of the submodes? If it
> > doesn't, then why not add a general init function-variable/hook/whatever
> > into which initialisations can be plugged?
> It does not pick up each and every hook.
> If it did, though, it would only call your before-change-functions
> inside js-mode regions, but it would have ignored them in HTML and CSS
> regions. Which doesn't appear to be what you want anyway.
Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?
It's not very nice, but it helps to analyse in the abstract how we
reached the point we are at. That abstract reason is js-mode using part
of CC Mode without initialising it. This is bound to lead to trouble,
and it has lead to trouble.
> >> And, in general, to have per-mode before-change-functions contents.
> > There's no problem with before/after-change-functions. They're the
> > canonical way to react to buffer changes.
> They're not very manageable, from mmm's point of view. And like the
> current example shows, it's not obvious what to do with such hooks
> outside of submode regions of major modes that added them.
Like I said earlier on in the thread, making several major modes in a
buffer work is problematic in Emacs, and we really want better support
from the C core for it. Here we seem to want "global" and "mode-local"
before-change-functionses.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 19:13 ` Alan Mackenzie
@ 2020-06-25 19:28 ` Dmitry Gutov
2020-06-25 20:11 ` Alan Mackenzie
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-25 19:28 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
On 25.06.2020 22:13, Alan Mackenzie wrote:
>> That's unfortunate.
> Indeed. Let's assume that keeping it working is a requirement here.
Still, buffers that user mixed modes are usually not so big as some of
the files we have in src/*.c. So even forgoing caching might result in a
satisfying user experience 98% of the time.
>> Guess the only thing that remains for me here is to express a wish for a
>> syntax-ppss based design here.
>
>> Because mmm-mode knows how to deal with major modes based on it, as a group.
>
> How about enhancing mmm-mode to handle any major mode, rather than a
> restricted subset?
I don't know how. before-change-functions don't make it easy.
>> It does not pick up each and every hook.
>
>> If it did, though, it would only call your before-change-functions
>> inside js-mode regions, but it would have ignored them in HTML and CSS
>> regions. Which doesn't appear to be what you want anyway.
>
> Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
> js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?
That would be something every user that configures a submode class using
js-mode have to be aware of. That's not easy to document, or even if we
made sure it's documented, to be sure that users read it.
>>> There's no problem with before/after-change-functions. They're the
>>> canonical way to react to buffer changes.
>
>> They're not very manageable, from mmm's point of view. And like the
>> current example shows, it's not obvious what to do with such hooks
>> outside of submode regions of major modes that added them.
>
> Like I said earlier on in the thread, making several major modes in a
> buffer work is problematic in Emacs, and we really want better support
> from the C core for it. Here we seem to want "global" and "mode-local"
> before-change-functionses.
These do seem to be the options: some C core support (though I'm not
clear on the particulars of the proposed design), or switching from
ad-hoc caches to syntax-propertize-function and and associated
syntax-ppss cache.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 19:28 ` Dmitry Gutov
@ 2020-06-25 20:11 ` Alan Mackenzie
2020-06-25 21:20 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-25 20:11 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Simen Heggestøyl, acm, 41897
Hello, Dmitry.
On Thu, Jun 25, 2020 at 22:28:17 +0300, Dmitry Gutov wrote:
> On 25.06.2020 22:13, Alan Mackenzie wrote:
> >> That's unfortunate.
> > Indeed. Let's assume that keeping it working is a requirement here.
> Still, buffers that user mixed modes are usually not so big as some of
> the files we have in src/*.c. So even forgoing caching might result in
> a satisfying user experience 98% of the time.
Sluggish performance isn't about "usually" and 98% of the time; it's
about unusual constellations and the other 2%.
[ .... ]
> >> If it did, though, it would only call your before-change-functions
> >> inside js-mode regions, but it would have ignored them in HTML and CSS
> >> regions. Which doesn't appear to be what you want anyway.
> > Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
> > js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?
> That would be something every user that configures a submode class using
> js-mode have to be aware of. That's not easy to document, or even if we
> made sure it's documented, to be sure that users read it.
Are you telling me that mmm-mode couldn't keep a watch out for js-mode,
leaving other libraries untroubled? Again, the trouble here appears to
arise from using something (a mode) without first initialising it.
> >>> There's no problem with before/after-change-functions. They're the
> >>> canonical way to react to buffer changes.
> >> They're not very manageable, from mmm's point of view. And like the
> >> current example shows, it's not obvious what to do with such hooks
> >> outside of submode regions of major modes that added them.
> > Like I said earlier on in the thread, making several major modes in a
> > buffer work is problematic in Emacs, and we really want better
> > support from the C core for it. Here we seem to want "global" and
> > "mode-local" before-change-functionses.
> These do seem to be the options: some C core support (though I'm not
> clear on the particulars of the proposed design), or switching from
> ad-hoc caches to syntax-propertize-function and and associated
> syntax-ppss cache.
The syntax-propertize-function approach is poor design. It restricts the
use of the syntax-table text property too much. syntax-ppss has had a
troubled history and doesn't do the right thing in narrowed buffers. It
advertises itself as a magic wand which does everything, but when you've
been enticed into committing your SW to it, you then find out it's less
than magic, and you've got to call ugly functions by hand at strange
times, and are restricted in how and when you can use it.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-22 19:17 ` Alan Mackenzie
2020-06-23 0:02 ` Dmitry Gutov
@ 2020-06-25 20:49 ` Tom Tromey
2020-06-26 16:46 ` Alan Mackenzie
2020-07-04 13:13 ` Alan Mackenzie
2 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2020-06-25 20:49 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
>>>>> "Alan" == Alan Mackenzie <acm@muc.de> writes:
Alan> (ii) An enhancement to CC Mode, used by mhtml-mode and js-mode, which
Alan> invalidates a CC Mode cache used by the filling code. This gets rid of
Alan> a few strange looking bugs;
I wonder if js-mode can be changed not to use cc-mode like this.
Looking at js.el, it seems like this is the origin of a use of advice
in-core, which is generally frowned upon:
;; FIXME: Such redefinitions are bad style. We should try and use some other
;; way to get the same result.
(defun js--fill-c-advice (js-fun)
...
Tom
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-23 19:17 ` Alan Mackenzie
2020-06-23 23:11 ` Dmitry Gutov
@ 2020-06-25 20:53 ` Tom Tromey
2020-06-25 21:14 ` Dmitry Gutov
2020-06-26 16:31 ` Alan Mackenzie
1 sibling, 2 replies; 30+ messages in thread
From: Tom Tromey @ 2020-06-25 20:53 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897, Dmitry Gutov
>>>>> "Alan" == Alan Mackenzie <acm@muc.de> writes:
Alan> Having several major modes in a single buffer has always been problematic
Alan> in Emacs. Personally, I think there needs to be amendments in the
Alan> low-level C code to support it properly, but I'm not able to do this work
Alan> on my own, and there doesn't seem to be enough enthusiasm on other
Alan> people's part to help out.
mhtml-mode was a stab at making this work. It does require some more
changes in Emacs (there's another bug open about font-lock that requires
some changes to the font-lock code, but there are also a few more I
could list), but I didn't come across anything requiring C changes.
What changes are you thinking of?
Dmitry> TBH I'm not sure if mhtml-mode does the save-and-restore dance which
Dmitry> would be necessary for this. mmm-mode does, though.
It does. Whatever happened to the idea of pulling mmm into Emacs core?
Maybe we could get rid of mhtml-mode.
Tom
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 20:53 ` Tom Tromey
@ 2020-06-25 21:14 ` Dmitry Gutov
2020-06-26 16:31 ` Alan Mackenzie
1 sibling, 0 replies; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-25 21:14 UTC (permalink / raw)
To: Tom Tromey, Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
On 25.06.2020 23:53, Tom Tromey wrote:
> Dmitry> TBH I'm not sure if mhtml-mode does the save-and-restore dance which
> Dmitry> would be necessary for this. mmm-mode does, though.
>
> It does.
When moving between regions?
> Whatever happened to the idea of pulling mmm into Emacs core?
> Maybe we could get rid of mhtml-mode.
It's in ELPA now, which is a good place to be.
I'm maintaining it to keep it usable, but depending on your end goals,
it might require a more significant rewrite. Help welcome.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 20:11 ` Alan Mackenzie
@ 2020-06-25 21:20 ` Dmitry Gutov
2020-06-27 11:06 ` Alan Mackenzie
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-25 21:20 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
On 25.06.2020 23:11, Alan Mackenzie wrote:
> Sluggish performance isn't about "usually" and 98% of the time; it's
> about unusual constellations and the other 2%.
Still, a slow-ish fill-paragraph is nowhere near as bad as, say,
slowdown during typing.
>>> Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
>>> js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?
>
>> That would be something every user that configures a submode class using
>> js-mode have to be aware of. That's not easy to document, or even if we
>> made sure it's documented, to be sure that users read it.
>
> Are you telling me that mmm-mode couldn't keep a watch out for js-mode,
> leaving other libraries untroubled? Again, the trouble here appears to
> arise from using something (a mode) without first initialising it.
Sounds like special-casing js-mode, before-change-functions and this
particular function all together. Basically, like a magic constant in
the code.
This is ultimately doable, but I'm not sure how to write a patch for it
which wouldn't leave me feeling dirty after.
>> These do seem to be the options: some C core support (though I'm not
>> clear on the particulars of the proposed design), or switching from
>> ad-hoc caches to syntax-propertize-function and and associated
>> syntax-ppss cache.
>
> The syntax-propertize-function approach is poor design. It restricts the
> use of the syntax-table text property too much. syntax-ppss has had a
> troubled history and doesn't do the right thing in narrowed buffers. It
> advertises itself as a magic wand which does everything, but when you've
> been enticed into committing your SW to it, you then find out it's less
> than magic, and you've got to call ugly functions by hand at strange
> times, and are restricted in how and when you can use it.
Despite certain edge cases, I've had a lot of success with it. Both in
major modes, and in mmm-mode thanks to it.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 20:53 ` Tom Tromey
2020-06-25 21:14 ` Dmitry Gutov
@ 2020-06-26 16:31 ` Alan Mackenzie
1 sibling, 0 replies; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-26 16:31 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simen Heggestøyl, 41897, Dmitry Gutov
Hello, Tom.
On Thu, Jun 25, 2020 at 14:53:44 -0600, Tom Tromey wrote:
> >>>>> "Alan" == Alan Mackenzie <acm@muc.de> writes:
> Alan> Having several major modes in a single buffer has always been problematic
> Alan> in Emacs. Personally, I think there needs to be amendments in the
> Alan> low-level C code to support it properly, but I'm not able to do this work
> Alan> on my own, and there doesn't seem to be enough enthusiasm on other
> Alan> people's part to help out.
> mhtml-mode was a stab at making this work. It does require some more
> changes in Emacs (there's another bug open about font-lock that requires
> some changes to the font-lock code, but there are also a few more I
> could list), but I didn't come across anything requiring C changes.
> What changes are you thinking of?
In 2016, I came up with a preliminary, but detailed, design for
"islands" which would make several major modes per buffer natively and
naturally supported, without awkward workarounds. This included
"island-local" variables to supplement buffer-local variables. All this
would be implemented at the C level and would obviate the awkward
pre/post-command-hook functions currently used by mhtml-mode for
swapping in and out the buffer local variables.
The post to emacs-devel which started the discussion was:
Subject: A vision for multiple major modes: some design notes
To: emacs-devel@gnu.org, Dmitry Gutov <dgutov@yandex.ru>
Date: Wed, 20 Apr 2016 19:44:50 +0000
Although interest was expressed, in the end there was nobody else
jumping up shouting "hey, that's great! Can I help?" (not that I
specifically asked for help), so the idea didn't advance beyond that
preliminary design. I still think that parts of it could be helpful in
mmm-mode (or its successor).
> Dmitry> TBH I'm not sure if mhtml-mode does the save-and-restore dance which
> Dmitry> would be necessary for this. mmm-mode does, though.
> It does. Whatever happened to the idea of pulling mmm into Emacs core?
> Maybe we could get rid of mhtml-mode.
> Tom
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 20:49 ` Tom Tromey
@ 2020-06-26 16:46 ` Alan Mackenzie
0 siblings, 0 replies; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-26 16:46 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simen Heggestøyl, acm, 41897
Hello, Tom.
On Thu, Jun 25, 2020 at 14:49:57 -0600, Tom Tromey wrote:
> >>>>> "Alan" == Alan Mackenzie <acm@muc.de> writes:
> Alan> (ii) An enhancement to CC Mode, used by mhtml-mode and js-mode, which
> Alan> invalidates a CC Mode cache used by the filling code. This gets rid of
> Alan> a few strange looking bugs;
> I wonder if js-mode can be changed not to use cc-mode like this.
The main filling function in CC Mode is 390 lines long. It deals with
lots of special cases, like comment markers being or not being on lines
by themselves, and so on. It's understandable to want to reuse it rather
than copying it into a js-... function.
> Looking at js.el, it seems like this is the origin of a use of advice
> in-core, which is generally frowned upon:
> ;; FIXME: Such redefinitions are bad style. We should try and use some other
> ;; way to get the same result.
> (defun js--fill-c-advice (js-fun)
> ...
Funnily enough, as CC Mode maintainer, I don't find this artifice
particularly objectionable. Advice has its uses, even in the core.
> Tom
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-25 21:20 ` Dmitry Gutov
@ 2020-06-27 11:06 ` Alan Mackenzie
2020-06-28 0:18 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Alan Mackenzie @ 2020-06-27 11:06 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Simen Heggestøyl, acm, 41897
Hello, Dmitry.
On Fri, Jun 26, 2020 at 00:20:07 +0300, Dmitry Gutov wrote:
> On 25.06.2020 23:11, Alan Mackenzie wrote:
> > Sluggish performance isn't about "usually" and 98% of the time; it's
> > about unusual constellations and the other 2%.
> Still, a slow-ish fill-paragraph is nowhere near as bad as, say,
> slowdown during typing.
We're talking about a long pause at the end of each line when typing in
a longish block comment whilst auto-fill-mode is enabled.
> >>> Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
> >>> js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?
> >> That would be something every user that configures a submode class using
> >> js-mode have to be aware of. That's not easy to document, or even if we
> >> made sure it's documented, to be sure that users read it.
> > Are you telling me that mmm-mode couldn't keep a watch out for js-mode,
> > leaving other libraries untroubled? Again, the trouble here appears to
> > arise from using something (a mode) without first initialising it.
> Sounds like special-casing js-mode, before-change-functions and this
> particular function all together. Basically, like a magic constant in
> the code.
> This is ultimately doable, but I'm not sure how to write a patch for it
> which wouldn't leave me feeling dirty after.
If I understand correctly, there is already special case code for js-mode
anyway, in mmm-erb.el. The :creation-hook for js-mode is currently set
to mmm-erb-mark-as-special, a function shared with ruby-mode.
Would it really be all that distressing to write a new :creation-hook for
js-mode which additionally initialises the part of CC Mode which needs
it? We'd be talking about something like
(defun mmm-js-init ()
"Doc string."
(overlay-put mmm-current-overlay 'mmm-special-tag t)
(c-foreign-init-lit-pos-cache)
(add-hook 'before-change-functions #'c-foreign-truncate-lit-pos-cache nil t))
. Would that really be so bad? It would even be possible to move the
explicit add-hook into a small function in CC Mode and call that instead.
But I think it's better to see the add-hook where it might make a
difference.
That's all assuming I've understood mmm-mode properly.
[ .... ]
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-27 11:06 ` Alan Mackenzie
@ 2020-06-28 0:18 ` Dmitry Gutov
0 siblings, 0 replies; 30+ messages in thread
From: Dmitry Gutov @ 2020-06-28 0:18 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Simen Heggestøyl, 41897
On 27.06.2020 14:06, Alan Mackenzie wrote:
> Hello, Dmitry.
>
> On Fri, Jun 26, 2020 at 00:20:07 +0300, Dmitry Gutov wrote:
>> On 25.06.2020 23:11, Alan Mackenzie wrote:
>
>>> Sluggish performance isn't about "usually" and 98% of the time; it's
>>> about unusual constellations and the other 2%.
>
>> Still, a slow-ish fill-paragraph is nowhere near as bad as, say,
>> slowdown during typing.
>
> We're talking about a long pause at the end of each line when typing in
> a longish block comment whilst auto-fill-mode is enabled.
OK, that's an argument.
I'm inclined to believe it should be acceptably small for all such files
in practice, but I don't have any solid data to back that up.
>>>>> Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
>>>>> js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?
>
>>>> That would be something every user that configures a submode class using
>>>> js-mode have to be aware of. That's not easy to document, or even if we
>>>> made sure it's documented, to be sure that users read it.
>
>>> Are you telling me that mmm-mode couldn't keep a watch out for js-mode,
>>> leaving other libraries untroubled? Again, the trouble here appears to
>>> arise from using something (a mode) without first initialising it.
>
>> Sounds like special-casing js-mode, before-change-functions and this
>> particular function all together. Basically, like a magic constant in
>> the code.
>
>> This is ultimately doable, but I'm not sure how to write a patch for it
>> which wouldn't leave me feeling dirty after.
>
> If I understand correctly, there is already special case code for js-mode
> anyway, in mmm-erb.el. The :creation-hook for js-mode is currently set
> to mmm-erb-mark-as-special, a function shared with ruby-mode.
Good find, in a way: to satisfy your requirement, :creation-hook is
likely the property to use.
But the code you pointed to (mmm-erb.el) contains specialized major mode
that uses js-mode, among other modes, to implement support for ERB, EJS
and JSP templates, together with indentation logic.
It is also the only built-in example of a major mode in mmm-mode.
Whereas people use js-mode also in other contexts, a lot of the time in
configurations that they create themselves (called submode classes,
which are then associated with existing major modes, to be used as
"primary" mode, and file extensions).
As such, any user who creates such a configuration involving js-mode
will need to be aware of c-foreign-init-lit-pos-cache and
c-foreign-truncate-lit-pos-cache.
> Would it really be all that distressing to write a new :creation-hook for
> js-mode which additionally initialises the part of CC Mode which needs
> it? We'd be talking about something like
>
> (defun mmm-js-init ()
> "Doc string."
> (overlay-put mmm-current-overlay 'mmm-special-tag t)
> (c-foreign-init-lit-pos-cache)
> (add-hook 'before-change-functions #'c-foreign-truncate-lit-pos-cache nil t))
It's not by itself distressing as long the one who needs to write it
remembers and understands its necessity.
In any case, I tried this solution together with the patch you attached
(assuming patching the code and evaluating the new contents of
cc-engine.el should be enough to make it work), but it didn't help with
the same scenario in html-erb-mode. It didn't make anything worse, though.
Since mmm-mode is actually out of scope of this bug report, though,
perhaps I should stop with this thread of discussion.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode
2020-06-22 19:17 ` Alan Mackenzie
2020-06-23 0:02 ` Dmitry Gutov
2020-06-25 20:49 ` Tom Tromey
@ 2020-07-04 13:13 ` Alan Mackenzie
2 siblings, 0 replies; 30+ messages in thread
From: Alan Mackenzie @ 2020-07-04 13:13 UTC (permalink / raw)
To: Simen Heggestøyl; +Cc: Tom Tromey, 41897-done, Dmitry Gutov
Hello, Simen.
On Mon, Jun 22, 2020 at 19:17:50 +0000, Alan Mackenzie wrote:
> We're gradually converging on working code. Since yesterday, the
> improvements are:
> (i) (slightly) better handling of auto-fill-mode, which is no longer
> automatically enabled;
> (ii) An enhancement to CC Mode, used by mhtml-mode and js-mode, which
> invalidates a CC Mode cache used by the filling code. This gets rid of
> a few strange looking bugs;
> (iii) The inclusion of "paragraph-" in the "crucial variables" thing, so
> that paragraph-start and paragraph-separate from js-mode get into
> mhtml-mode.
I've committed more or less the patch from my post of 2020-06-22, which
I'm convinced fixes the bug. I'm closing the bug with this post.
[ .... ]
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2020-07-04 13:13 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <874krbaqg3.fsf@simenheg@gmail.com>
[not found] ` <mailman.1991.1592327403.2541.bug-gnu-emacs@gnu.org>
2020-06-20 17:18 ` bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode Alan Mackenzie
2020-06-20 18:27 ` Simen Heggestøyl
[not found] ` <87d05ta8z9.fsf@simenheg@gmail.com>
2020-06-21 16:55 ` Alan Mackenzie
2020-06-22 19:17 ` Alan Mackenzie
2020-06-23 0:02 ` Dmitry Gutov
2020-06-23 8:36 ` Alan Mackenzie
2020-06-23 14:23 ` Dmitry Gutov
2020-06-23 16:28 ` Alan Mackenzie
2020-06-23 17:59 ` Dmitry Gutov
2020-06-23 19:17 ` Alan Mackenzie
2020-06-23 23:11 ` Dmitry Gutov
2020-06-24 17:43 ` Alan Mackenzie
2020-06-24 18:28 ` Dmitry Gutov
2020-06-25 16:33 ` Alan Mackenzie
2020-06-25 16:48 ` Dmitry Gutov
2020-06-25 18:07 ` Alan Mackenzie
2020-06-25 18:19 ` Dmitry Gutov
2020-06-25 19:13 ` Alan Mackenzie
2020-06-25 19:28 ` Dmitry Gutov
2020-06-25 20:11 ` Alan Mackenzie
2020-06-25 21:20 ` Dmitry Gutov
2020-06-27 11:06 ` Alan Mackenzie
2020-06-28 0:18 ` Dmitry Gutov
2020-06-25 20:53 ` Tom Tromey
2020-06-25 21:14 ` Dmitry Gutov
2020-06-26 16:31 ` Alan Mackenzie
2020-06-25 20:49 ` Tom Tromey
2020-06-26 16:46 ` Alan Mackenzie
2020-07-04 13:13 ` Alan Mackenzie
2020-06-16 17:08 Simen Heggestøyl
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).