* lisp/progmodes/octave.el: Use grammar more; Handle enumeration fun

Remove redundant :group keyword args.
(octave-begin-keywords, octave-else-keywords, octave-end-keywords):
Remove variables.
(octave-operator-table, octave-smie-bnf-table): Use let-when-compile to
turn them into compile-time variables.
Auto-generate the "foo ... end" rules from the "foo ... endfoo" rules.
Add rules for break, continue, return, global, and persistent.
Refine the rule for "until".
(octave-smie--funcall-p, octave-smie--end-index-p)
(octave-smie--in-parens-p): New functions.
(octave-smie-backward-token, octave-smie-forward-token): Use them to
distinguish the "enumeration" function and the "end" index from
their corresponding keywords.
(octave--block-offset-keywords): New constant.
(octave-smie-rules): Use it.  Adjust rules for new global/persistent parsing.
(octave-reserved-words): Redefine using octave-smie-grammar.
(octave-font-lock-keywords): Use octave-smie--funcall-p and
octave-smie--end-index-p.

2 files changed, 186 insertions, 152 deletions

diff --git a/lisp/progmodes/octave.el b/lisp/progmodes/octave.el
index 70a2b1ab5ad..b54b88dccb5 100644
--- a/lisp/progmodes/octave.el
+++ b/lisp/progmodes/octave.el
@@ -82,25 +82,6 @@ Used in `octave-mode' and `inferior-octave-mode' buffers.")
 (defvar octave-comment-start-skip "\\(^\\|\\S<\\)\\(?:%!\\|\\s<+\\)\\s-*"
   "Octave-specific `comment-start-skip' (which see).")
 
-(defvar octave-begin-keywords
-  '("classdef" "do" "enumeration" "events" "for" "function" "if" "methods"
-    "parfor" "properties" "switch" "try" "unwind_protect" "while"))
-
-(defvar octave-else-keywords
-  '("case" "catch" "else" "elseif" "otherwise" "unwind_protect_cleanup"))
-
-(defvar octave-end-keywords
-  '("endclassdef" "endenumeration" "endevents" "endfor" "endfunction" "endif"
-    "endmethods" "endparfor" "endproperties" "endswitch" "end_try_catch"
-    "end_unwind_protect" "endwhile" "until" "end"))
-
-(defvar octave-reserved-words
-  (append octave-begin-keywords
-          octave-else-keywords
-          octave-end-keywords
-          '("break" "continue" "global" "persistent" "return"))
-  "Reserved words in Octave.")
-
 (defvar octave-function-header-regexp
   (concat "^\\s-*\\_<\\(function\\)\\_>"
           "\\([^=;(\n]*=[ \t]*\\|[ \t]*\\)\\(\\(?:\\w\\|\\s_\\)+\\)\\_>")
@@ -231,20 +212,17 @@ parenthetical grouping.")
 (defcustom octave-font-lock-texinfo-comment t
   "Control whether to highlight the texinfo comment block."
   :type 'boolean
-  :group 'octave
   :version "24.4")
 
 (defcustom octave-blink-matching-block t
   "Control the blinking of matching Octave block keywords.
 Non-nil means show matching begin of block when inserting a space,
 newline or semicolon after an else or end keyword."
-  :type 'boolean
-  :group 'octave)
+  :type 'boolean)
 
 (defcustom octave-block-offset 2
   "Extra indentation applied to statements in Octave block structures."
-  :type 'integer
-  :group 'octave)
+  :type 'integer)
 
 (defvar octave-block-comment-start
   (concat (make-string 2 octave-comment-char) " ")
@@ -252,8 +230,7 @@ newline or semicolon after an else or end keyword."
 
 (defcustom octave-continuation-offset 4
   "Extra indentation applied to Octave continuation lines."
-  :type 'integer
-  :group 'octave)
+  :type 'integer)
 
 (eval-and-compile
   (defconst octave-continuation-marker-regexp "\\\\\\|\\.\\.\\."))
@@ -274,109 +251,135 @@ newline or semicolon after an else or end keyword."
 
 (defcustom octave-mode-hook nil
   "Hook to be run when Octave mode is started."
-  :type 'hook
-  :group 'octave)
+  :type 'hook)
 
 (defcustom octave-send-show-buffer t
   "Non-nil means display `inferior-octave-buffer' after sending to it."
-  :type 'boolean
-  :group 'octave)
+  :type 'boolean)
 
 (defcustom octave-send-line-auto-forward t
   "Control auto-forward after sending to the inferior Octave process.
 Non-nil means always go to the next Octave code line after sending."
-  :type 'boolean
-  :group 'octave)
+  :type 'boolean)
 
 (defcustom octave-send-echo-input t
   "Non-nil means echo input sent to the inferior Octave process."
-  :type 'boolean
-  :group 'octave)
+  :type 'boolean)
 
 
 ;;; SMIE indentation
 
 (require 'smie)
 
-;; Use '__operators__' in Octave REPL to get a full list.
-(defconst octave-operator-table
-  '((assoc ";" "\n") (assoc ",") ; The doc claims they have equal precedence!?
-    (right "=" "+=" "-=" "*=" "/=")
-    (assoc "&&") (assoc "||") ; The doc claims they have equal precedence!?
-    (assoc "&") (assoc "|")   ; The doc claims they have equal precedence!?
-    (nonassoc "<" "<=" "==" ">=" ">" "!=" "~=")
-    (nonassoc ":")                      ;No idea what this is.
-    (assoc "+" "-")
-    (assoc "*" "/" "\\" ".\\" ".*" "./")
-    (nonassoc "'" ".'")
-    (nonassoc "++" "--" "!" "~")        ;And unary "+" and "-".
-    (right "^" "**" ".^" ".**")
-    ;; It's not really an operator, but for indentation purposes it
-    ;; could be convenient to treat it as one.
-    (assoc "...")))
-
-(defconst octave-smie-bnf-table
-  '((atom)
-    ;; We can't distinguish the first element in a sequence with
-    ;; precedence grammars, so we can't distinguish the condition
-    ;; if the `if' from the subsequent body, for example.
-    ;; This has to be done later in the indentation rules.
-    (exp (exp "\n" exp)
-         ;; We need to mention at least one of the operators in this part
-         ;; of the grammar: if the BNF and the operator table have
-         ;; no overlap, SMIE can't know how they relate.
-         (exp ";" exp)
-         ("try" exp "catch" exp "end_try_catch")
-         ("try" exp "catch" exp "end")
-         ("unwind_protect" exp
-          "unwind_protect_cleanup" exp "end_unwind_protect")
-         ("unwind_protect" exp "unwind_protect_cleanup" exp "end")
-         ("for" exp "endfor")
-         ("for" exp "end")
-         ("parfor" exp "endparfor")
-         ("parfor" exp "end")
-         ("do" exp "until" atom)
-         ("while" exp "endwhile")
-         ("while" exp "end")
-         ("if" exp "endif")
-         ("if" exp "else" exp "endif")
-         ("if" exp "elseif" exp "else" exp "endif")
-         ("if" exp "elseif" exp "elseif" exp "else" exp "endif")
-         ("if" exp "elseif" exp "elseif" exp "else" exp "end")
-         ("switch" exp "case" exp "endswitch")
-         ("switch" exp "case" exp "otherwise" exp "endswitch")
-         ("switch" exp "case" exp "case" exp "otherwise" exp "endswitch")
-         ("switch" exp "case" exp "case" exp "otherwise" exp "end")
-         ("function" exp "endfunction")
-         ("function" exp "end")
-         ("enumeration" exp "endenumeration")
-         ("enumeration" exp "end")
-         ("events" exp "endevents")
-         ("events" exp "end")
-         ("methods" exp "endmethods")
-         ("methods" exp "end")
-         ("properties" exp "endproperties")
-         ("properties" exp "end")
-         ("classdef" exp "endclassdef")
-         ("classdef" exp "end"))
-    ;; (fundesc (atom "=" atom))
-    ))
+(let-when-compile
+    ((operator-table
+      ;; Use '__operators__' in Octave REPL to get a full list?
+      '((assoc ";" "\n") (assoc ",") ;The doc says they have equal precedence!?
+        (right "=" "+=" "-=" "*=" "/=")
+        (assoc "&&") (assoc "||") ; The doc claims they have equal precedence!?
+        (assoc "&") (assoc "|") ; The doc claims they have equal precedence!?
+        (nonassoc "<" "<=" "==" ">=" ">" "!=" "~=")
+        (nonassoc ":")                  ;No idea what this is.
+        (assoc "+" "-")
+        (assoc "*" "/" "\\" ".\\" ".*" "./")
+        (nonassoc "'" ".'")
+        (nonassoc "++" "--" "!" "~")    ;And unary "+" and "-".
+        (right "^" "**" ".^" ".**")
+        ;; It's not really an operator, but for indentation purposes it
+        ;; could be convenient to treat it as one.
+        (assoc "...")))
+
+     (matchedrules
+      ;; We can't distinguish the first element in a sequence with
+      ;; precedence grammars, so we can't distinguish the condition
+      ;; of the `if' from the subsequent body, for example.
+      ;; This has to be done later in the indentation rules.
+      '(("try" exp "catch" exp "end_try_catch")
+        ("unwind_protect" exp
+         "unwind_protect_cleanup" exp "end_unwind_protect")
+        ("for" exp "endfor")
+        ("parfor" exp "endparfor")
+        ("while" exp "endwhile")
+        ("if" exp "endif")
+        ("if" exp "else" exp "endif")
+        ("if" exp "elseif" exp "else" exp "endif")
+        ("if" exp "elseif" exp "elseif" exp "else" exp "endif")
+        ("switch" exp "case" exp "endswitch")
+        ("switch" exp "case" exp "otherwise" exp "endswitch")
+        ("switch" exp "case" exp "case" exp "otherwise" exp "endswitch")
+        ("function" exp "endfunction")
+        ("enumeration" exp "endenumeration")
+        ("events" exp "endevents")
+        ("methods" exp "endmethods")
+        ("properties" exp "endproperties")
+        ("classdef" exp "endclassdef")
+        ))
+
+     (bnf-table
+      `((atom)
+        ;; FIXME: We don't parse these declarations correctly since
+        ;; SMIE *really* likes to parse "a b = 2 c" as "(a b) = (2 c)".
+        ;; IOW to do it right, we'd need to change octave-smie-*ward-token
+        ;; so that the spaces between vars in var-decls are lexed as
+        ;; something like ",".
+        ;; Doesn't seem worth the trouble/slowdown for now.
+        ;; We could hack smie-rules so as to work around the bad parse,
+        ;; but even that doesn't seem worth the trouble.
+        (var-decls (atom "=" atom)) ;; (var-decls "," var-decls)
+        (single-exp (atom "=" atom))
+        (exp (exp "\n" exp)
+             ;; We need to mention at least one of the operators in this part
+             ;; of the grammar: if the BNF and the operator table have
+             ;; no overlap, SMIE can't know how they relate.
+             (exp ";" exp)
+             ("do" exp "until" single-exp)
+             ,@matchedrules
+           ;; For every rule that ends in "endfoo", add a corresponding
+           ;; rule which uses "end" instead.
+           ,@(mapcar (lambda (rule) (nconc (butlast rule) '("end")))
+                     matchedrules)
+           ("global" var-decls) ("persistent" var-decls)
+           ;; These aren't super-important, but having them here
+           ;; makes it easier to extract all keywords.
+           ("break") ("continue") ("return")
+           ;; The following rules do not correspond to valid code AFAIK,
+           ;; but they lead to a grammar that degrades more gracefully
+           ;; on incomplete/incorrect code.  It also helps us in
+           ;; computing octave--block-offset-keywords.
+           ("try" exp "end") ("unwind_protect" exp "end")
+           )
+      ;; (fundesc (atom "=" atom))
+      )))
 
 (defconst octave-smie-grammar
-  (smie-prec2->grammar
-   (smie-merge-prec2s
-    (smie-bnf->prec2 octave-smie-bnf-table
-                     '((assoc "\n" ";")))
+  (eval-when-compile
+    (smie-prec2->grammar
+     (smie-merge-prec2s
+      (smie-bnf->prec2 bnf-table '((assoc "\n" ";")))
+      (smie-precs->prec2 operator-table)))))
 
-    (smie-precs->prec2 octave-operator-table))))
+(defconst octave-operator-regexp
+  (eval-when-compile
+    (regexp-opt (remove "\n" (apply #'append
+                                    (mapcar #'cdr operator-table)))))))
 
 ;; Tokenizing needs to be refined so that ";;" is treated as two
 ;; tokens and also so as to recognize the \n separator (and
 ;; corresponding continuation lines).
 
-(defconst octave-operator-regexp
-  (regexp-opt (remove "\n" (apply 'append
-                                  (mapcar 'cdr octave-operator-table)))))
+(defun octave-smie--funcall-p ()
+  "Return non-nil if we're in an expression context.  Moves point."
+  (looking-at "[ \t]*("))
+
+(defun octave-smie--end-index-p ()
+  (let ((ppss (syntax-ppss)))
+    (and (nth 1 ppss)
+         (memq (char-after (nth 1 ppss)) '(?\( ?\[ ?\{)))))
+
+(defun octave-smie--in-parens-p ()
+  (let ((ppss (syntax-ppss)))
+    (and (nth 1 ppss)
+         (eq ?\( (char-after (nth 1 ppss))))))
 
 (defun octave-smie-backward-token ()
   (let ((pos (point)))
@@ -390,10 +393,7 @@ Non-nil means always go to the next Octave code line after sending."
                  (forward-comment (- (point)))
                  nil)
              t)
-           ;; Ignore it if it's within parentheses.
-           (let ((ppss (syntax-ppss)))
-             (not (and (nth 1 ppss)
-                       (eq ?\( (char-after (nth 1 ppss)))))))
+           (not (octave-smie--in-parens-p)))
       (skip-chars-forward " \t")
       ;; Why bother distinguishing \n and ;?
       ";") ;;"\n"
@@ -403,7 +403,15 @@ Non-nil means always go to the next Octave code line after sending."
       (goto-char (match-beginning 0))
       (match-string-no-properties 0))
      (t
-      (smie-default-backward-token)))))
+      (let ((tok (smie-default-backward-token)))
+        (cond
+         ((equal tok "enumeration")
+          (if (save-excursion (smie-default-forward-token)
+                              (octave-smie--funcall-p))
+              "enumeration (function)"
+            tok))
+         ((equal tok "end") (if (octave-smie--end-index-p) "end (index)" tok))
+         (t tok)))))))
 
 (defun octave-smie-forward-token ()
   (skip-chars-forward " \t")
@@ -417,10 +425,7 @@ Non-nil means always go to the next Octave code line after sending."
          (not (or (save-excursion (skip-chars-backward " \t")
                                   ;; Only add implicit ; when needed.
                                   (or (bolp) (eq (char-before) ?\;)))
-                  ;; Ignore it if it's within parentheses.
-                  (let ((ppss (syntax-ppss)))
-                    (and (nth 1 ppss)
-                         (eq ?\( (char-after (nth 1 ppss))))))))
+                  (octave-smie--in-parens-p))))
     (if (eolp) (forward-char 1) (forward-comment 1))
     ;; Why bother distinguishing \n and ;?
     ";") ;;"\n"
@@ -436,7 +441,25 @@ Non-nil means always go to the next Octave code line after sending."
     (goto-char (match-end 0))
     (match-string-no-properties 0))
    (t
-    (smie-default-forward-token))))
+    (let ((tok (smie-default-forward-token)))
+      (cond
+       ((equal tok "enumeration")
+        (if (octave-smie--funcall-p)
+            "enumeration (function)"
+          tok))
+       ((equal tok "end") (if (octave-smie--end-index-p) "end (index)" tok))
+       (t tok))))))
+
+(defconst octave--block-offset-keywords
+  (let* ((end-prec (nth 1 (assoc "end" octave-smie-grammar)))
+         (end-matchers
+          (delq nil
+                (mapcar (lambda (x) (if (eq end-prec (nth 2 x)) (car x)))
+                        octave-smie-grammar))))
+    ;; Not sure if it would harm to keep "switch", but the previous code
+    ;; excluded it, presumably because there shouldn't be any code on
+    ;; the lines between "switch" and "case".
+    (delete "switch" end-matchers)))
 
 (defun octave-smie-rules (kind token)
   (pcase (cons kind token)
@@ -445,15 +468,12 @@ Non-nil means always go to the next Octave code line after sending."
     ;; - changes to octave-block-offset wouldn't take effect immediately.
     ;; - edebug wouldn't show the use of this variable.
     (`(:elem . basic) octave-block-offset)
+    (`(:list-intro . ,(or "global" "persistent")) t)
     ;; Since "case" is in the same BNF rules as switch..end, SMIE by default
     ;; aligns it with "switch".
     (`(:before . "case") (if (not (smie-rule-sibling-p)) octave-block-offset))
     (`(:after . ";")
-     (if (smie-rule-parent-p "classdef" "events" "enumeration" "function" "if"
-                             "while" "else" "elseif" "for" "parfor"
-                             "properties" "methods" "otherwise" "case"
-                             "try" "catch" "unwind_protect"
-                             "unwind_protect_cleanup")
+     (if (apply #'smie-rule-parent-p octave--block-offset-keywords)
          (smie-rule-parent octave-block-offset)
        ;; For (invalid) code between switch and case.
        ;; (if (smie-rule-parent-p "switch") 4)
@@ -473,28 +493,33 @@ Non-nil means always go to the next Octave code line after sending."
       (comment-choose-indent)))))
 
 
+(defvar octave-reserved-words
+  (delq nil
+        (mapcar (lambda (x)
+                  (setq x (car x))
+                  (and (stringp x) (string-match "\\`[[:alpha:]]" x) x))
+                octave-smie-grammar))
+  "Reserved words in Octave.")
+
 (defvar octave-font-lock-keywords
   (list
    ;; Fontify all builtin keywords.
-   (cons (concat "\\_<\\("
-                 (regexp-opt octave-reserved-words)
-                 "\\)\\_>")
+   (cons (concat "\\_<" (regexp-opt octave-reserved-words) "\\_>")
          'font-lock-keyword-face)
-   ;; Note: 'end' also serves as the last index in an indexing expression.
+   ;; Note: 'end' also serves as the last index in an indexing expression,
+   ;; and 'enumerate' is also a function.
    ;; Ref: http://www.mathworks.com/help/matlab/ref/end.html
+   ;; Ref: http://www.mathworks.com/help/matlab/ref/enumeration.html
    (list (lambda (limit)
-           (while (re-search-forward "\\_<end\\_>" limit 'move)
+           (while (re-search-forward "\\_<en\\(?:d\\|umeratio\\(n\\)\\)\\_>"
+                                     limit 'move)
              (let ((beg (match-beginning 0))
                    (end (match-end 0)))
                (unless (octave-in-string-or-comment-p)
-                 (condition-case nil
-                     (progn
-                       (goto-char beg)
-                       (backward-up-list)
-                       (when (memq (char-after) '(?\( ?\[ ?\{))
-                         (put-text-property beg end 'face nil))
-                       (goto-char end))
-                   (error (goto-char end))))))
+                 (when (if (match-end 1)
+                           (octave-smie--funcall-p)
+                         (octave-smie--end-index-p))
+                   (put-text-property beg end 'face nil)))))
            nil))
    ;; Fontify all operators.
    (cons octave-operator-regexp 'font-lock-builtin-face)
@@ -609,27 +634,23 @@ Key bindings:
 
 (defcustom inferior-octave-program "octave"
   "Program invoked by `inferior-octave'."
-  :type 'string
-  :group 'octave)
+  :type 'string)
 
 (defcustom inferior-octave-buffer "*Inferior Octave*"
   "Name of buffer for running an inferior Octave process."
-  :type 'string
-  :group 'octave)
+  :type 'string)
 
 (defcustom inferior-octave-prompt
   ;; For Octave >= 3.8, default is always 'octave', see
   ;; http://hg.savannah.gnu.org/hgweb/octave/rev/708173343c50
   "\\(?:^octave\\(?:.bin\\|.exe\\)?\\(?:-[.0-9]+\\)?\\(?::[0-9]+\\)?\\|^debug\\|^\\)>+ "
   "Regexp to match prompts for the inferior Octave process."
-  :type 'regexp
-  :group 'octave)
+  :type 'regexp)
 
 (defcustom inferior-octave-prompt-read-only comint-prompt-read-only
   "If non-nil, the Octave prompt is read only.
 See `comint-prompt-read-only' for details."
   :type 'boolean
-  :group 'octave
   :version "24.4")
 
 (defcustom inferior-octave-startup-file
@@ -639,7 +660,6 @@ See `comint-prompt-read-only' for details."
 The contents of this file are sent to the inferior Octave process on
 startup."
   :type '(choice (const :tag "None" nil) file)
-  :group 'octave
   :version "24.4")
 
 (defcustom inferior-octave-startup-args '("-i" "--no-line-editing")
@@ -647,13 +667,11 @@ startup."
 For example, for suppressing the startup message and using `traditional'
 mode, include \"-q\" and \"--traditional\"."
   :type '(repeat string)
-  :group 'octave
   :version "24.4")
 
 (defcustom inferior-octave-mode-hook nil
   "Hook to be run when Inferior Octave mode is started."
-  :type 'hook
-  :group 'octave)
+  :type 'hook)
 
 (defcustom inferior-octave-error-regexp-alist
   '(("error:\\s-*\\(.*?\\) at line \\([0-9]+\\), column \\([0-9]+\\)"
@@ -663,8 +681,7 @@ mode, include \"-q\" and \"--traditional\"."
   "Value for `compilation-error-regexp-alist' in inferior octave."
   :version "24.4"
   :type '(repeat (choice (symbol :tag "Predefined symbol")
-                         (sexp :tag "Error specification")))
-  :group 'octave)
+                         (sexp :tag "Error specification"))))
 
 (defvar inferior-octave-compilation-font-lock-keywords
   '(("\\_<PASS\\_>" . compilation-info-face)
@@ -995,7 +1012,6 @@ directory and makes this the current buffer's default directory."
 (defcustom inferior-octave-minimal-columns 80
   "The minimal column width for the inferior Octave process."
   :type 'integer
-  :group 'octave
   :version "24.4")
 
 (defvar inferior-octave-last-column-width nil)
@@ -1180,8 +1196,7 @@ q: Don't fix\n" func file))
 
 (defface octave-function-comment-block
   '((t (:inherit font-lock-doc-face)))
-  "Face used to highlight function comment block."
-  :group 'octave)
+  "Face used to highlight function comment block.")
 
 (eval-when-compile (require 'texinfo))
 
@@ -1602,7 +1617,6 @@ code line."
   :type '(choice (const :tag "Automatic" auto)
                  (const :tag "One Line" oneline)
                  (const :tag "Multi Line" multiline))
-  :group 'octave
   :version "24.4")
 
 ;; (FN SIGNATURE1 SIGNATURE2 ...)
@@ -1661,7 +1675,6 @@ code line."
 (defcustom octave-help-buffer "*Octave Help*"
   "Buffer name for `octave-help'."
   :type 'string
-  :group 'octave
   :version "24.4")
 
 ;; Used in a mode derived from help-mode.
@@ -1786,7 +1799,6 @@ sentence."
   "A list of directories for Octave sources.
 If the environment variable OCTAVE_SRCDIR is set, it is searched first."
   :type '(repeat directory)
-  :group 'octave
   :version "24.4")
 
 (defun octave-source-directories ()
diff --git a/test/indent/octave.m b/test/indent/octave.m
index a7041462f7f..4758f9933cb 100644
--- a/test/indent/octave.m
+++ b/test/indent/octave.m
@@ -1,6 +1,19 @@
 ## -*- mode: octave; coding: utf-8 -*-
 0;                              # Don't make this a function file
 function res = tcomp (fn)
+
+  global x y ...
+         z1 z2
+  persistent x y ...
+             z1 z2
+  global x y = 2 ...
+         z1 z2                  # FIXME
+
+  do
+    something
+  until x = ...
+        y
+
   %% res = tcomp (fn)
   %%     imports components and rearranges them.
 
@@ -10,6 +23,15 @@ function res = tcomp (fn)
 
   data = dlmread(fn, 3, 0);
 
+  enumeration
+    first (1)
+    second (2)
+  end
+
+  y = enumeration (x);          #Beware: "enumeration" can also be a function!
+  y = foo(enumeration (x),
+          2);          #Beware: "enumeration" can also be a function!
+
   x = data(:,2:end);
   y = 'hello';
   z = y';