Package: emacs;
Reported by: Simen Heggestøyl <simenheg <at> runbox.com>
Date: Tue, 16 Jun 2020 17:10:01 UTC
Severity: normal
Found in version 28.0.50
Done: Alan Mackenzie <acm <at> muc.de>
Bug is archived. No further changes may be made.
Message #14 received at 41897 <at> debbugs.gnu.org (full text, mbox):
From: Alan Mackenzie <acm <at> muc.de> To: Simen Heggestøyl <simenheg <at> runbox.com> Cc: 41897 <at> debbugs.gnu.org Subject: Re: bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode Date: Sun, 21 Jun 2020 16:55:13 +0000
Hello again, Simen.
On Sat, Jun 20, 2020 at 20:27:22 +0200, Simen Heggestøyl wrote:
> Alan Mackenzie <acm <at> 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).
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.