mmm-mode icon indicating copy to clipboard operation
mmm-mode copied to clipboard

No font-locking with sql-mode as the submode

Open purcell opened this issue 12 years ago • 10 comments

I use the following to highlight blocks of SQL inside ruby heredocs:

(eval-after-load 'mmm-mode
  '(progn
     (mmm-add-classes
      '((ruby-heredoc-sql
         :submode sql-mode
         :front "<<-?[\'\"]?\\(end_sql\\)[\'\"]?"
         :save-matches 1
         :front-offset (end-of-line 1)
         :back "^[ \t]*~1$"
         :delimiter-mode nil)))
     (mmm-add-mode-ext-class 'ruby-mode "\\.rb\\'" 'ruby-heredoc-sql)))

An example would be

<<-end_sql
SELECT foo FROM bar;
end_sql

The SQL always shows up in the ruby string color, though. :-/

purcell avatar Mar 06 '13 15:03 purcell

Did it look differently, before my syntax-propertize-function changes?

dgutov avatar Mar 06 '13 23:03 dgutov

Good question... I'll take a look and report back.

purcell avatar Mar 07 '13 08:03 purcell

The result seems to be the same even before those changes, at least when I try loading up old revisions of mmm-mode.el and mmm-region.el in the same Emacs instance, which should be adequate.

purcell avatar Mar 07 '13 13:03 purcell

(I tested e414b5b, for example, from 6 months ago.)

purcell avatar Mar 07 '13 13:03 purcell

sql-mode is weird. Do you get this one-line snippet fontified when it's the only text in the buffer, in sql-mode? I don't. The tricks below allow to fontify ruby-mode subregions within strings in html-mode, but not sql-mode in ruby-mode heredocs.

We can either:

  1. Update font-lock-keywords for select submodes, change (some of) the keywords to use override. This is most unobtrusive (you can use derived modes), but the mode must do font-lock straghtforwardly enough, and the strings inside the submode will also have keywords, etc, highlighted, obviously.

  2. Trick syntax-ppss to never use cache in subregions, this may have some performance impact, and this way we have to also ignore strings that may have originated in the previous region of current submode (the case of nested submodes, as with my 100 bottles example in on the last pull requests). Proof of concept:

diff --git a/mmm-region.el b/mmm-region.el
index 384be68..62d3f6e 100644
--- a/mmm-region.el
+++ b/mmm-region.el
@@ -779,7 +779,15 @@ of the REGIONS covers START to STOP."
                   (mmm-set-local-variables (unless (eq mmm-previous-submode mode)
                                              mode)
                                            mmm-current-overlay)
-                  (funcall func (car reg) (cadr reg) nil)
+                  (save-restriction
+                    (if mmm-current-overlay
+                        (narrow-to-region (overlay-start mmm-current-overlay)
+                                          (overlay-end mmm-current-overlay)))
+                    (let ((font-lock-dont-widen t)
+                          syntax-ppss-cache
+                          ;; syntax-ppss-last
+                          )
+                        (funcall func (car reg) (cadr reg) nil)))
                   ;; Catch changes in font-lock cache.
                   (mmm-save-changed-local-variables
                    mmm-current-submode mmm-current-overlay))

While syntax-ppss-last is commented out, the 100 bottles example still works, and my favorite big test file also looks fine, but this is likely to break in some case.

To have it both ways, I think we could add an additional argument to subregion class definitions, like font-lock-use-narrowing. And use it for sql and other heredoc-type subregions.

dgutov avatar Mar 09 '13 22:03 dgutov

The SQL snippet is fontified correctly when it's alone in an sql-mode buffer. That's in a recent Emacs HEAD snapshot.

I'm not sure I understand enough to express a preference between your options 1 and 2, but the combination of narrowing and suppressing of the syntax cache intuitively looks like a good idea to me.

purcell avatar Mar 10 '13 13:03 purcell

On 10.03.2013 17:37, Steve Purcell wrote:

The SQL snippet is fontified correctly when it's alone in an sql-mode buffer. That's in a recent Emacs HEAD snapshot.

Still doesn't work for me, with -Q or without, in different Emacs versions. And `font-lock-keywords' is nil in such buffers.

I'm not sure I understand enough to express a preference between your options 1 and 2, but the combination of narrowing and suppressing of the syntax cache intuitively looks like a good idea to me.

I'll try to figure out the problem with sql-mode, meanwhile you can try the above proof-of-concept patch with your .rb files, see if it breaks in some cases.

dgutov avatar Mar 10 '13 15:03 dgutov

Ah, so sql-mode subregions aren't highlighted even with this patch. We don't get the value of font-lock-keywords saved for sql-mode, for some reason. The why is harder to tell, sql-product-font-lock looks complicated.

dgutov avatar Mar 12 '13 04:03 dgutov

It looks like this issue hasn't seen any activity in a while, but I thought it might be helpful if I mentioned that I have a mostly-working config for embedding sql-mode in php-mode under emacs 23 via mmm-mode.

There are some issues with recognizing the end of a SQL block (sometimes breaks font-lock after the sql-mode string), but the SQL statement itself is highlighted correctly. It works with current master, and looks like this:

(setq php-sql-mmm-submode-enabled nil)
(defun php-sql-mmm-submode ()
  "Provides a very minimal embedding of SQL in PHP, via mmm-mode."
  (interactive)
  (when (not php-sql-mmm-submode-enabled)
    (set-face-background 'mmm-default-submode-face nil)
    (mmm-add-classes
     '((embedded-sql
        :submode sql-mode
        :front "$\\(sql\\|query\\) = \""
        :back "\";"
        :face mmm-code-submode-face)))
    (mmm-add-mode-ext-class 'php-mode "\\.php$" 'embedded-sql)
    (setq php-sql-mmm-submode-enabled t)))

which I then call from my php-mode hook.

Under Emacs 24.3, which I've recently started moving to, SQL is mostly displayed with font-lock-warning-face. Interestingly, though, SQL comments seem to be displayed with the right face. Evidently something changed between 23 and 24.3 to make this break.

I'm not much of an elisper, so I'm not quite sure how I'd start digging into this further. If there's anything else I can do that would be helpful, please let me know and I'll see what I can do.

NateEag avatar Sep 09 '13 03:09 NateEag

Thanks for the extra data points Nate. Hope to get around to fixing this one day.

purcell avatar Sep 09 '13 07:09 purcell