exposing the effective mode in a multi-mode

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

exposing the effective mode in a multi-mode

Tom Tromey-4
I noticed that company couldn't do CSS completion in a <style> element
in mhtml-mode.

company-css understands how web-mode handles sub-modes, by calling a
web-mode function ("web-mode-language-at-pos") directly, but it doesn't
understand mhtml-mode.

So, I'd like to propose this patch for Emacs 26.  It adds a new function
to prog-mode.el so that multi-modes can expose the sub-mode at point.
It also changes mhtml-mode to set the new variable to make this work.

Tom

diff --git a/lisp/progmodes/prog-mode.el b/lisp/progmodes/prog-mode.el
index f727e45..88d6e59 100644
--- a/lisp/progmodes/prog-mode.el
+++ b/lisp/progmodes/prog-mode.el
@@ -96,6 +96,24 @@ prog-indentation-context
    A typical use case are literate programming sources -- the
    function would successively return the previous code chunks.")
 
+(defvar-local prog-effective-mode-function nil
+  "When non-nil, provides the effective mode at point for embedded code chunks.
+When non-nil, this is a function that will be called by
+`prog-effective-mode' to find the effective major mode at point.
+This function should return a symbol naming a major mode, e.g. `css-mode'.
+It may return nil to mean the \"outer\" major mode.")
+
+(defun prog-effective-mode ()
+  "When non-nil, returns the effective mode at point.
+
+There are languages where part of the code is actually written in
+a sub language, e.g., a Yacc/Bison or ANTLR grammar also consists
+of plain C code.  This function returns the effective value of
+`major-mode' at point."
+  (or (if prog-effective-mode-function
+          (funcall 'prog-effective-mode-function))
+      major-mode))
+
 (defun prog-indent-sexp (&optional defun)
   "Indent the expression after point.
 When interactively called with prefix, indent the enclosing defun
diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
index b6cd157..2953355 100644
--- a/lisp/textmodes/mhtml-mode.el
+++ b/lisp/textmodes/mhtml-mode.el
@@ -56,7 +56,9 @@ mhtml-tag-relative-indent
   :version "26.1")
 
 (cl-defstruct mhtml--submode
-  ;; Name of this submode.
+  ;; The submode's symbol, like 'css-mode.
+  mode
+  ;; Name of this submode, a string.  Used for the mode-line lighter.
   name
   ;; HTML end tag.
   end-tag
@@ -86,7 +88,7 @@ mhtml--construct-submode
   "A wrapper for make-mhtml--submode that computes the buffer-local variables."
   (let ((captured-locals nil)
         (crucial-captured-locals nil)
-        (submode (apply #'make-mhtml--submode args)))
+        (submode (apply #'make-mhtml--submode :mode mode args)))
     (with-temp-buffer
       (funcall mode)
       ;; Make sure font lock is all set up.
@@ -348,6 +350,14 @@ mhtml--flyspell-check-word
         (flyspell-generic-progmode-verify)
       t)))
 
+(defun mhtml--effective-mode ()
+  "Return the current mode (a symbol) at point.
+If point is not in a submode section, returns 'html-mode."
+  (let ((submode (get-text-property (point) 'mhtml-submode)))
+    (if submode
+        (mhtml--submode-mode submode)
+      'html-mode)))
+
 ;;;###autoload
 (define-derived-mode mhtml-mode html-mode
   '((sgml-xml-mode "XHTML+" "HTML+") (:eval (mhtml--submode-lighter)))
@@ -365,6 +375,7 @@ mhtml-mode
   (setq-local font-lock-extend-region-functions
               '(mhtml--extend-font-lock-region
                 font-lock-extend-region-multiline))
+  (setq-local prog-effective-mode-function 'mhtml--effective-mode)
 
   ;; Attach this to both pre- and post- hooks just in case it ever
   ;; changes a key binding that might be accessed from the menu bar.

Reply | Threaded
Open this post in threaded view
|

Re: exposing the effective mode in a multi-mode

Dmitry Gutov
On 9/18/17 2:13 AM, Tom Tromey wrote:

> I noticed that company couldn't do CSS completion in a <style> element
> in mhtml-mode.
>
> company-css understands how web-mode handles sub-modes, by calling a
> web-mode function ("web-mode-language-at-pos") directly, but it doesn't
> understand mhtml-mode.
>
> So, I'd like to propose this patch for Emacs 26.  It adds a new function
> to prog-mode.el so that multi-modes can expose the sub-mode at point.
> It also changes mhtml-mode to set the new variable to make this work.

Looks good to me, but consider this: sometimes (not too often, though)
we have several major modes corresponding to a language. and they don't
inherit from one another. Example: perl-mode and cperl-mode.

So I thought it might be useful to return the name of the language
instead of the mode. The default implementation could look like this:

(defun prog-effective-lang ()
   (intern (replace-regexp-in-string "-mode\\'" ""
                                     (symbol-name major-mode))))

The "extra" major modes would have to provide their own implementations,
though. Although define-derived-mode could grow a new keyword parameter,
to make this easier.

> +(defvar-local prog-effective-mode-function nil
> +  "When non-nil, provides the effective mode at point for embedded code chunks.
> +When non-nil, this is a function that will be called by
> +`prog-effective-mode' to find the effective major mode at point.
> +This function should return a symbol naming a major mode, e.g. `css-mode'.
> +It may return nil to mean the \"outer\" major mode.")
> +
> +(defun prog-effective-mode ()
> +  "When non-nil, returns the effective mode at point.

Can it return nil? When?

Reply | Threaded
Open this post in threaded view
|

Re: exposing the effective mode in a multi-mode

Alan Mackenzie
In reply to this post by Tom Tromey-4
Hello, Tom

On Sun, Sep 17, 2017 at 17:13:39 -0600, Tom Tromey wrote:
> I noticed that company couldn't do CSS completion in a <style> element
> in mhtml-mode.

> company-css understands how web-mode handles sub-modes, by calling a
> web-mode function ("web-mode-language-at-pos") directly, but it doesn't
> understand mhtml-mode.

> So, I'd like to propose this patch for Emacs 26.  It adds a new function
> to prog-mode.el so that multi-modes can expose the sub-mode at point.
> It also changes mhtml-mode to set the new variable to make this work.

prog-mode seems the wrong place to put this.  It has nothing to do with
programming languages, as such, and everything to do with multi-mode
modes.

The facility will surely be needed by modes which aren't derived from
prog-mode.

The implementation of multi modes is, as far as I understand it, still
in flux.  Is the current implementation going to be applicable to all
these various ways of doing multi-mode?  Would it not be better to have
this as a hook function (in global namespace), where the pertinent
multi-mode could set the hook to its own function?

Just a few thoughts.

> Tom

[ .... ]

--
Alan Mackenzie (Nuremberg, Germany).

Reply | Threaded
Open this post in threaded view
|

Re: exposing the effective mode in a multi-mode

Stefan Monnier
In reply to this post by Tom Tromey-4
> +(defvar-local prog-effective-mode-function nil
> +  "When non-nil, provides the effective mode at point for embedded code chunks.
> +When non-nil, this is a function that will be called by
> +`prog-effective-mode' to find the effective major mode at point.
> +This function should return a symbol naming a major mode, e.g. `css-mode'.
> +It may return nil to mean the \"outer\" major mode.")
> +
> +(defun prog-effective-mode ()
> +  "When non-nil, returns the effective mode at point.
> +
> +There are languages where part of the code is actually written in
> +a sub language, e.g., a Yacc/Bison or ANTLR grammar also consists
> +of plain C code.  This function returns the effective value of
> +`major-mode' at point."
> +  (or (if prog-effective-mode-function
> +          (funcall 'prog-effective-mode-function))
> +      major-mode))

As usual, I recommend to avoid the "when non-nil, ..." for function
variables (e.g. because it prevents use of add-function on it, and
because removing the nil case simplifies the code, and because removing
the nil case makes sure that the default behavior is not special).
[ Also the doc shouldn't talk about who calls it, but what it does.  ]

    (defvar-local prog-effective-mode-function #'ignore
      "Function to get the effective mode at point for embedded code chunks.
    Called with no argument, returns the effective major mode at point.
    This function should return a symbol naming a major mode, e.g. `css-mode'.
    It may return nil to mean the \"outer\" major mode.")
   
    (defun prog-effective-mode ()
      "Return the effective mode at point.
    There are languages where part of the code is actually written in
    a sub language, e.g., a Yacc/Bison or ANTLR grammar also consists
    of plain C code.  This function returns the effective value of
    `major-mode' at point."
      (or (funcall 'prog-effective-mode-function)
          major-mode))

Tho we can simplify it yet further:

    (defvar-local prog-effective-mode-function (lambda () major-mode)
      "Function to get the effective mode at point for embedded code chunks.
    Called with no argument, returns the effective major mode at point.
    This function should return a symbol naming a major mode, e.g. `css-mode'.")
   
    (defun prog-effective-mode ()
      "Return the effective mode at point.
    There are languages where part of the code is actually written in
    a sub language, e.g., a Yacc/Bison or ANTLR grammar also consists
    of plain C code.  This function returns the effective value of
    `major-mode' at point."
       (funcall 'prog-effective-mode-function))

at which point we may as well drop `prog-effective-mode`.


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: exposing the effective mode in a multi-mode

Tom Tromey-4
In reply to this post by Dmitry Gutov
>>>>> "Dmitry" == Dmitry Gutov <[hidden email]> writes:

Dmitry> So I thought it might be useful to return the name of the language
Dmitry> instead of the mode. The default implementation could look like this:

Are there modes that can adapt to multiple different major modes for
language, but without knowing the details of the major mode?  I don't
know but it seems iffy to me.

Dmitry> Can it return nil? When?

Copy-paste bug, I've fixed the comment.  Thanks.

Tom

Reply | Threaded
Open this post in threaded view
|

Re: exposing the effective mode in a multi-mode

Tom Tromey-4
In reply to this post by Alan Mackenzie
>>>>> "Alan" == Alan Mackenzie <[hidden email]> writes:

Alan> The facility will surely be needed by modes which aren't derived from
Alan> prog-mode.

That's a good point, and I've updated the patch accordingly.  It's
appended; let me know what you think.

Alan> Is the current implementation going to be applicable to all
Alan> these various ways of doing multi-mode?  Would it not be better to have
Alan> this as a hook function (in global namespace), where the pertinent
Alan> multi-mode could set the hook to its own function?

I don't really understand this critique, because that's what the patch
did.  Is there something I should change?

Tom

diff --git a/lisp/subr.el b/lisp/subr.el
index 79ae1f4..5c63f4f 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1906,6 +1906,24 @@ derived-mode-p
   "Non-nil if the current major mode is derived from one of MODES.
 Uses the `derived-mode-parent' property of the symbol to trace backwards."
   (apply #'provided-mode-derived-p major-mode modes))
+
+(defvar-local effective-major-mode-function nil
+  "When non-nil, provides the effective mode at point for embedded chunks.
+When non-nil, this is a function that will be called by
+`effective-major-mode' to find the effective major mode at point.
+This function should return a symbol naming a major mode, e.g. `css-mode'.
+It may return nil to mean the \"outer\" major mode.")
+
+(defun effective-major-mode ()
+  "Returns the effective mode at point.
+
+There are languages where part of the code is actually written in
+a sub language, e.g., a Yacc/Bison or ANTLR grammar also consists
+of plain C code.  This function returns the effective value of
+`major-mode' at point."
+  (or (if effective-major-mode-function
+          (funcall effective-major-mode-function))
+      major-mode))
 
 ;;;; Minor modes.
 
diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
index b6cd157..326cee4 100644
--- a/lisp/textmodes/mhtml-mode.el
+++ b/lisp/textmodes/mhtml-mode.el
@@ -56,7 +56,9 @@ mhtml-tag-relative-indent
   :version "26.1")
 
 (cl-defstruct mhtml--submode
-  ;; Name of this submode.
+  ;; The submode's symbol, like 'css-mode.
+  mode
+  ;; Name of this submode, a string.  Used for the mode-line lighter.
   name
   ;; HTML end tag.
   end-tag
@@ -86,7 +88,7 @@ mhtml--construct-submode
   "A wrapper for make-mhtml--submode that computes the buffer-local variables."
   (let ((captured-locals nil)
         (crucial-captured-locals nil)
-        (submode (apply #'make-mhtml--submode args)))
+        (submode (apply #'make-mhtml--submode :mode mode args)))
     (with-temp-buffer
       (funcall mode)
       ;; Make sure font lock is all set up.
@@ -348,6 +350,13 @@ mhtml--flyspell-check-word
         (flyspell-generic-progmode-verify)
       t)))
 
+(defun mhtml--effective-mode ()
+  "Return the current mode (a symbol) at point.
+If point is not in a submode section, returns 'html-mode."
+  (let ((submode (get-text-property (point) 'mhtml-submode)))
+    (when submode
+      (mhtml--submode-mode submode))))
+
 ;;;###autoload
 (define-derived-mode mhtml-mode html-mode
   '((sgml-xml-mode "XHTML+" "HTML+") (:eval (mhtml--submode-lighter)))
@@ -365,6 +374,7 @@ mhtml-mode
   (setq-local font-lock-extend-region-functions
               '(mhtml--extend-font-lock-region
                 font-lock-extend-region-multiline))
+  (setq-local effective-major-mode-function 'mhtml--effective-mode)
 
   ;; Attach this to both pre- and post- hooks just in case it ever
   ;; changes a key binding that might be accessed from the menu bar.

Reply | Threaded
Open this post in threaded view
|

Re: exposing the effective mode in a multi-mode

Dmitry Gutov
In reply to this post by Tom Tromey-4
On 9/20/17 7:18 AM, Tom Tromey wrote:
>>>>>> "Dmitry" == Dmitry Gutov <[hidden email]> writes:
>
> Dmitry> So I thought it might be useful to return the name of the language
> Dmitry> instead of the mode. The default implementation could look like this:
>
> Are there modes that can adapt to multiple different major modes for
> language, but without knowing the details of the major mode?  I don't
> know but it seems iffy to me.

Do you know many cases where this isn't true?

Its the usual case in company-mode backends, I think. E.g.,
company-clang only cares that the edited language is one from a given
list, not about the current major mode. Same for third-party backends
for JS, and company-css, of course.

There might be some problems about unknown major modes setting the
syntax table to unreasonable values, but we could treat those as bugs in
those major modes.

Anyway, my suggestion makes things more complex. I'm not seeing a lot of
reaction to it, so maybe disregard for now. We could add some
major-mode->language mapping later down the road.

Reply | Threaded
Open this post in threaded view
|

Re: exposing the effective mode in a multi-mode

Tom Tromey-4
In reply to this post by Stefan Monnier
>>>>> "Stefan" == Stefan Monnier <[hidden email]> writes:

Stefan> at which point we may as well drop `prog-effective-mode`.

I didn't commit this patch because I'm no longer sure it's the best
approach.  It might be better to instead have mhtml-mode save
`major-mode' and set that when point enters a sub-mode region.  This way
other helping code would not have to be updated to find the effective
mode.

Tom