dgutov / mmm-mode

New official home for mmm-mode, fixed for Emacs >= 23
http://mmm-mode.sourceforge.net/
GNU General Public License v2.0
332 stars 30 forks source link

Need to save c-current-comment-prefix by region #100

Closed garyo closed 4 years ago

garyo commented 4 years ago

In a .vue file (using vue-mode based on mmm-mode), in the typescript section, if you have a multiline comment like this:

  /**
   |

and you hit TAB there (at the |), you'll get an error about nil not being a string. This is because indent is looking for c-current-comment-prefix but it's nil at that point. I fixed this by doing:

  (add-to-list 'mmm-save-local-variables '(c-current-comment-prefix region))

so perhaps mmm-mode should save this by region instead of globally as it does now.

dgutov commented 4 years ago

Global means it's associated with a major mode. c-current-comment-prefix is assigned only once: in c-setup-paragraph-variables. So that should work, but the entry in mmm-save-local-variables is only for mmm-c-derived-modes.

So please try the patch below (and restart Emacs).

Since js-mode is actually not derived from CC Mode, the proper solution might be to only do this for a select number of variables -- ones set in c-setup-paragraph-variables which js-mode calls. If you can write down and post this list, we can try an alternative patch as well.

diff --git a/mmm-vars.el b/mmm-vars.el
index 073a4b5..837fdcb 100644
--- a/mmm-vars.el
+++ b/mmm-vars.el
@@ -108,7 +108,7 @@
 ;;{{{ Save Local Variables

 (defvar mmm-c-derived-modes
-  '(c-mode c++-mode objc-mode pike-mode java-mode jde-mode javascript-mode
+  '(c-mode c++-mode objc-mode pike-mode java-mode jde-mode js-mode
     php-mode))

 (defvar mmm-save-local-variables
garyo commented 4 years ago

Actually this is in a typescript-mode section. Here's a test file:

<template>
<h1>
  hi
</h1>
</template>

<script lang="ts">
import * as d3 from 'd3'
function fooFunction(alpha: string, beta: number) {
  return 123 + alpha + beta
}
/**
 * |
 */
function bar() {
  fooFunction("123", 123)
  d3.interpolate("123", true)
}
</script>

<style lang="scss">
something {
    color: blue;
}
</style>

I tried your patch but it didn't help (as expected I guess because I didn't give all the info). Typescript-mode is derived from prog-mode, not cc-mode. But it does call into cc-mode. And it does set up some cc-mode vars (typescript-mode.el:2852):

  (let ((c-buffer-is-cc-mode t))
    ;; FIXME: These are normally set by `c-basic-common-init'.  Should
    ;; we call it instead?  (Bug#6071)
    (make-local-variable 'paragraph-start)
    (make-local-variable 'paragraph-separate)
    (make-local-variable 'paragraph-ignore-fill-prefix)
    (make-local-variable 'adaptive-fill-mode)
    (make-local-variable 'adaptive-fill-regexp)
    (c-setup-paragraph-variables))

Looking at your patch and explanation, actually I think all that's needed is to add typescript-mode to your list. I just tried that and that change actually does seem to work for me.

For completeness, here's the full backtrace of the error:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  looking-at(nil)
  c-lineup-C-comments((c . 167))
  c-evaluate-offset(c-lineup-C-comments (c . 167) c)
  c-calc-offset((c . 167))
  c-get-syntactic-indentation(((c . 167)))
  typescript--get-c-offset(c 167)
  typescript--proper-indentation((0 nil 135 nil t nil 0 nil 167 nil nil))
  typescript-indent-line()
  vue-mmm-indent-line-narrowed()
  indent--funcall-widened(vue-mmm-indent-line-narrowed)
  indent-for-tab-command(nil)
  funcall-interactively(indent-for-tab-command nil)
  call-interactively(indent-for-tab-command nil nil)
  command-execute(indent-for-tab-command)
dgutov commented 4 years ago

Thanks for the extra info. Guess we need to add both (I tested the example with js-mode and saw that problem).

dgutov commented 4 years ago

Somewhat relatedly, I'm still seeing problems with default-indent-new-line (bound to M-j), but there's not much to be done there because it stumbles on c-block-comment-ender-regexp being nil, but neither js-mode nor typescript-mode (I'm assuming) don't have this variable set.

dgutov commented 4 years ago

Anyway, @garyo please let me know when you're moderately satisfied with how things work, and I'll cut a new release.

dgutov commented 4 years ago

Related: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=11165

(We should really identify and save the relevant vars).

garyo commented 4 years ago

It seems to be working fine for me now, thanks!

On Wed, Nov 6, 2019 at 5:32 PM Dmitry Gutov notifications@github.com wrote:

Related: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=11165

(We should really identify and save the relevant vars).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/purcell/mmm-mode/issues/100?email_source=notifications&email_token=AABCFRYTPCC4FTVZS3HYNLTQSNAXLA5CNFSM4JGOSREKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDIHBOA#issuecomment-550531256, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABCFR7CKD5LJIQDJENDPHTQSNAXLANCNFSM4JGOSREA .

-- Gary