Rework indentation

The previous implementation did not support some legal syntax:

* spaces between rule lines

* indented rule blocks (e.g., a rule defined under an if-statement)

* top-level commands like "include" when the value started on the second
  line (re: #16)

2 files changed, 215 insertions, 366 deletions

diff --git a/snakemake-mode.el b/snakemake-mode.el
index 96d577c..cd8a421 100644
--- a/snakemake-mode.el
+++ b/snakemake-mode.el
@@ -47,6 +47,7 @@
 
 ;;; Code:
 
+(require 'cl-lib)
 (require 'python)
 
 
@@ -73,7 +74,9 @@
 ;;; Regexp
 
 (defconst snakemake-rule-or-subworkflow-re
-  (rx (or (and (group symbol-start (or "rule" "subworkflow"))
+  (rx line-start
+      (zero-or-more space)
+      (or (and (group symbol-start (or "rule" "subworkflow"))
                " "
                (group (one-or-more (or (syntax word) (syntax symbol))))
                (zero-or-more space)
@@ -83,10 +86,6 @@
                ":")))
   "Regexp matching a rule or subworkflow.")
 
-(defconst snakemake-rule-or-subworkflow-line-re
-  (concat "^" snakemake-rule-or-subworkflow-re)
-  "Regexp matching a rule or subworkflow at start of line.")
-
 (defconst snakemake-toplevel-command-re
   (rx line-start
       (zero-or-more space)
@@ -135,97 +134,95 @@
 
 ;;; Indentation
 
-(defun snakemake-indent-line ()
-  "Indent the current line.
-Outside of rule blocks, handle indentation as it would be in a
-Python mode buffer (using `python-indent-line-function').  Inside
-rule blocks (or on a blank line directly below), call
-`snakemake-indent-rule-line'."
-  (interactive)
-  (if (snakemake-in-rule-or-subworkflow-block-p)
-      (snakemake-indent-rule-line)
-    (python-indent-line-function)))
-
-(defun snakemake-indent-rule-line ()
-  "Indent line of a rule or subworkflow block.
-
-Indent according the the first case below that is true.
-
-- At the top of rule block
-
-  Remove all indentation.
-
-- At a rule field key ('input', 'output',...) or on the first
-  field line of the block
-
-  Indent the line to `snakemake-indent-field-offset'.
-
-- On first line below a naked field key.
-
-  Indent the line with `snakemake-indent-field-offset' plus
-  `snakemake-indent-value-offset'.
-
-- On any 'run' field value line except for the first value line.
-
-  Indent according to Python mode.
-
-- Before the current indentation
-
-  Move point to the current indentation.
-
-- Otherwise
-
-  Cycle between indenting to `snakemake-indent-field-offset',
-  indenting to the column of the previous field value.  If within
-  a field value for a naked field key, add a step that indents
-  according to Python mode."
-  (let ((start-col (current-column))
-        (start-indent (current-indentation)))
-    (save-excursion
-      (beginning-of-line)
-      (cond
-       ((looking-at-p (concat "^\\s-*" snakemake-rule-or-subworkflow-re))
-        (delete-horizontal-space))
-       ((or (looking-at-p (concat "^\\s-*" snakemake-field-key-re))
-            (snakemake-first-field-line-p))
-        (delete-horizontal-space)
-        (indent-to snakemake-indent-field-offset))
-       ((snakemake-below-naked-field-p)
-        (delete-horizontal-space)
-        (indent-to (+ snakemake-indent-field-offset
-                      snakemake-indent-value-offset)))
-       ((snakemake-run-field-line-p)
-        (python-indent-line-function))
-       ((>= start-col start-indent)
-        (let ((prev-col (snakemake-previous-field-value-column)))
-          (when prev-col
+(defun snakemake--calculate-indentation (&optional previous)
+  "Return indentation offset for the current line.
+
+A non-nil value for PREVIOUS indicates that the previous command
+was an indentation command.
+
+When Python mode should handle the indentation, a nil value is
+returned."
+  (when (memq (car (python-indent-context))
+              (list :after-line
+                    ;; If point is on a value line following a naked
+                    ;; field value, `python-indent-context' returns
+                    ;; :after-block-start.
+                    :after-block-start))
+    (let* ((initial-indent (current-indentation))
+           (first-col (prog-first-column))
+           (goto-first-p (or (not previous) (= initial-indent first-col))))
+      (save-excursion
+        (save-restriction
+          (prog-widen)
+          (beginning-of-line)
+          (if (or (looking-at-p (concat "^\\s-*" snakemake-field-key-re))
+                  (looking-at-p (rx line-start
+                                    (zero-or-more space)
+                                    (or "\"\"\"" "'''"))))
+              (and goto-first-p
+                   (let (rule-indent)
+                     (while (not (or rule-indent (bobp)))
+                       (forward-line -1)
+                       (when (looking-at-p snakemake-rule-or-subworkflow-re)
+                         (setq rule-indent (current-indentation))))
+                     (and rule-indent
+                          (+ rule-indent snakemake-indent-field-offset))))
+            ;; We need to look back to determine indentation.
+            (skip-chars-backward " \t\n")
+            (beginning-of-line)
             (cond
-             ((and (snakemake-naked-field-line-p)
-                   (or (and (= start-indent 0)
-                            (not (looking-at-p "^\\s-*$")))
-                       (= start-indent prev-col)))
-              (let (last-command)
-                ;; ^ Don't let `python-indent-line' do clever things
-                ;; when indent command is repeated.
-                (python-indent-line-function))
-              (when (= (current-column) start-indent)
-                (delete-horizontal-space)
-                (indent-to snakemake-indent-value-offset)))
-             ((= start-indent snakemake-indent-field-offset)
-              (delete-horizontal-space)
-              (indent-to prev-col))
-             (t
-              (delete-horizontal-space)
-              (indent-to snakemake-indent-field-offset))))))))
-    (when (< (current-column) (current-indentation))
-      (forward-to-indentation 0))))
-
-(defun snakemake-first-field-line-p ()
-  "Return non-nil if point is on first field line of block."
-  (save-excursion
-    (forward-line -1)
-    (beginning-of-line)
-    (looking-at-p snakemake-rule-or-subworkflow-re)))
+             ((cl-some (lambda (re) (looking-at-p (concat re "\\s-*$")))
+                       (list snakemake-field-key-indented-re
+                             snakemake-rule-or-subworkflow-re
+                             snakemake-toplevel-command-re))
+              (let ((above-indent (current-indentation)))
+                (cond (goto-first-p
+                       (+ above-indent snakemake-indent-value-offset))
+                      ((< above-indent initial-indent)
+                       above-indent))))
+             ((looking-at (concat snakemake-field-key-indented-re "\\s-*"))
+              (let ((above-indent (current-indentation)))
+                (cond (goto-first-p
+                       (- (match-end 0) (line-beginning-position)))
+                      ((< above-indent initial-indent)
+                       above-indent))))
+             ((save-excursion
+                (let ((above-indent (current-indentation))
+                      field-indent)
+                  (when (> above-indent first-col)
+                    (while (and (not (bobp))
+                                (or (= above-indent
+                                       (setq field-indent (current-indentation)))
+                                    (looking-at-p "^\\s-*$")))
+                      (forward-line -1)))
+                  (and (looking-at
+                        (concat snakemake-field-key-indented-re "\\s-*"))
+                       (not (equal (match-string-no-properties 1)
+                                   "run"))
+                       (cond (goto-first-p
+                              (- (match-end 0) (line-beginning-position)))
+                             ((< field-indent initial-indent)
+                              field-indent)))))))))))))
+
+(defun snakemake-indent-line (&optional previous)
+  "Snakemake mode variant of `python-indent-line'."
+  (let ((follow-indentation-p
+         (and (<= (line-beginning-position) (point))
+              (>= (+ (line-beginning-position)
+                     (current-indentation))
+                  (point)))))
+    (save-excursion
+      (indent-line-to
+       (or (snakemake--calculate-indentation previous)
+           (python-indent-calculate-indentation previous))))
+    (when follow-indentation-p
+      (back-to-indentation))))
+
+(defun snakemake-indent-line-function ()
+  "Snakemake mode variant of `python-indent-line-function'."
+  (snakemake-indent-line
+   (and (memq this-command python-indent-trigger-commands)
+        (eq last-command this-command))))
 
 (defun snakemake-in-rule-or-subworkflow-block-p ()
   "Return non-nil if point is in block or on first blank line following one."
@@ -245,65 +242,6 @@ Indent according the the first case below that is true.
                  (throw 'in-block nil)))
           (forward-line -1))))))
 
-(defun snakemake-below-naked-field-p ()
-  "Return non-nil if point is on first line below a naked field key."
-  (save-excursion
-    (forward-line -1)
-    (beginning-of-line)
-    (looking-at-p (concat snakemake-field-key-indented-re "\\s-*$"))))
-
-(defun snakemake-naked-field-line-p ()
-  "Return non-nil if point is on any line of naked field key.
-This function assumes that point is in a rule or subworkflow
-block (which includes being on a blank line immediately below a
-block)."
-  (save-excursion
-    (let ((rule-start (save-excursion
-                        (end-of-line)
-                        (re-search-backward snakemake-rule-or-subworkflow-re
-                                            nil t))))
-      (end-of-line)
-      (and (re-search-backward snakemake-field-key-indented-re
-                               rule-start t)
-           (goto-char (match-end 0))
-           (looking-at-p "\\s-*$")))))
-
-(defun snakemake-run-field-line-p ()
-  "Return non-nil if point is on any line below a run field key.
-This function assumes that point is in a rule or subworkflow
-block (which includes being on a blank line immediately below a
-block).  If it's not, it gives the wrong answer if below a rule
-block whose last field is 'run'."
-  (save-excursion
-    (let ((rule-start (save-excursion
-                        (end-of-line)
-                        (re-search-backward snakemake-rule-or-subworkflow-re
-                                            nil t))))
-      (forward-line -1)
-      (end-of-line)
-      (re-search-backward snakemake-field-key-indented-re rule-start t)
-      (string= (match-string 1) "run"))))
-
-(defun snakemake-previous-field-value-column ()
-  "Get column for previous field value.
-
-If directly below a field key, this corresponds to the column for
-the first non-blank character after 'key:'.  Otherwise, it is the
-column of the first non-blank character.
-
-This function assumes that the previous line is a field value (in
-other words, that point is at or beyond the third line of a rule
-or subworkflow block."
-  (save-excursion
-    (forward-line -1)
-    (beginning-of-line)
-    ;; Because of multiline fields, the previous line may not have a
-    ;; key.
-    (let ((rule-re (concat "\\(?:" snakemake-field-key-indented-re
-                           "\\)*\\s-*\\S-")))
-      (when (re-search-forward rule-re (point-at-eol) t)
-        (1- (current-column))))))
-
 
 ;;; Imenu
 
@@ -324,7 +262,7 @@ label."
 (defun snakemake--imenu-build-rule-index ()
   (goto-char (point-min))
   (let (index)
-    (while (re-search-forward snakemake-rule-or-subworkflow-line-re nil t)
+    (while (re-search-forward snakemake-rule-or-subworkflow-re nil t)
       (push (cons (match-string-no-properties 2)
                   (save-excursion (beginning-of-line)
                                   (point-marker)))
@@ -385,7 +323,7 @@ embedded R, you need to set mmm-global-mode to a non-nil value such as 'maybe.")
 ;;; Mode
 
 (defvar snakemake-font-lock-keywords
-  `((,snakemake-rule-or-subworkflow-line-re
+  `((,snakemake-rule-or-subworkflow-re
      (1 font-lock-keyword-face nil 'lax)
      (2 font-lock-function-name-face nil 'lax)
      (3 font-lock-keyword-face nil 'lax))
@@ -398,7 +336,7 @@ embedded R, you need to set mmm-global-mode to a non-nil value such as 'maybe.")
   "Mode for editing Snakemake files."
   (set (make-local-variable 'imenu-create-index-function)
        #'snakemake-imenu-create-index)
-  (set (make-local-variable 'indent-line-function) 'snakemake-indent-line)
+  (set (make-local-variable 'indent-line-function) 'snakemake-indent-line-function)
   (set (make-local-variable 'indent-region-function) nil)
   (set (make-local-variable 'font-lock-defaults)
        `(,(append snakemake-font-lock-keywords python-font-lock-keywords))))
diff --git a/snakemake-test.el b/snakemake-test.el
index f092756..034350c 100644
--- a/snakemake-test.el
+++ b/snakemake-test.el
@@ -96,7 +96,6 @@ rule:
 ;;;; Indentation
 
 (ert-deftest snakemake-test-indent-line/at-rule-block ()
-  ;; Always shift first line of block to column 0.
   (should
    (string=
     "rule abc:"
@@ -118,8 +117,6 @@ rule:
         "     rule abc :"
       (snakemake-indent-line)
       (buffer-string))))
-
-  ;; Don't move point if beyond column 0.
   (should
    (string=
     "rule abc:  "
@@ -129,14 +126,28 @@ rule:
       (buffer-string))))
   (should
    (string=
-    "rule "
+    "
+if True:
+    rule abc:"
     (snakemake-with-temp-text
-        "    rule <point>abc:  <point>"
+        "
+if True:
+rule abc:<point>"
       (snakemake-indent-line)
-      (buffer-substring (point-min) (point))))))
+      (buffer-string))))
+  (should
+   (string=
+    "
+if True:
+rule abc:"
+    (snakemake-with-temp-text
+        "
+if True:
+    <point>rule abc:"
+      (snakemake-indent-line 'prev)
+      (buffer-string)))))
 
 (ert-deftest snakemake-test-indent-line/outside-rule ()
-  ;; Use standard Python mode indentation outside of rule blocks.
   (should
    (string=
     "
@@ -149,10 +160,49 @@ def ok():
       (snakemake-indent-line)
       (buffer-string)))))
 
+(ert-deftest snakemake-test-indent-line/toplevel-command ()
+  (should
+   (string=
+    "include: \"somefile\""
+    (snakemake-with-temp-text
+        "include: \"somefile\""
+      (snakemake-indent-line)
+      (buffer-string))))
+  (should
+   (string=
+    "include: \"somefile\""
+    (snakemake-with-temp-text
+        "   include: \"somefile\""
+      (snakemake-indent-line)
+      (buffer-string))))
+  (should
+   (string=
+    "
+if True:
+    include: \"somefile\"
+"
+    (snakemake-with-temp-text
+        "
+if True:
+<point>include: \"somefile\"
+"
+      (snakemake-indent-line)
+      (buffer-string))))
+  (should
+   (string=
+    "
+include:
+    \"somefile\"
+"
+    (snakemake-with-temp-text
+        "
+include:
+<point>\"somefile\"
+"
+      (snakemake-indent-line)
+      (buffer-string)))))
+
 (ert-deftest snakemake-test-indent-line/field-key ()
-  ;; Always indent first line to `snakemake-indent-field-offset'.
-  ;; Move point to `snakemake-indent-field-offset' if it is before any
-  ;; text on the line.
   (should
    (string=
     "
@@ -168,22 +218,58 @@ rule abc:
    (string=
     "
 rule abc:
+
     "
     (snakemake-with-temp-text
         "
 rule abc:
+
 <point>"
       (snakemake-indent-line)
-      (snakemake-indent-line)
       (buffer-string))))
   (should
    (string=
     "
 rule abc:
+    "
+    (snakemake-with-temp-text
+        "
+rule abc:
+<point>"
+      (snakemake-indent-line 'prev)
+      (buffer-string))))
+  (should
+   (string=
+    "
+rule abc:
+"
+    (snakemake-with-temp-text
+        "
+rule abc:
+   <point>"
+      (snakemake-indent-line 'prev)
+      (buffer-string))))
+  (should
+   (string=
+    "
+rule abc:
+    text"
+    (snakemake-with-temp-text
+        "
+rule abc:
+text<point>"
+      (snakemake-indent-line)
+      (buffer-substring (point-min) (point)))))
+  (should
+   (string=
+    "
+rule abc:
+
     text"
     (snakemake-with-temp-text
         "
 rule abc:
+
 text<point>"
       (snakemake-indent-line)
       (buffer-substring (point-min) (point)))))
@@ -198,10 +284,6 @@ rule abc:
 te<point>xt"
       (snakemake-indent-line)
       (buffer-substring (point-min) (point)))))
-
-  ;; Always indent field key to `snakemake-indent-field-offset'.
-  ;; Move point to `snakemake-indent-field-offset' if it is before any
-  ;; text on the line.
   (should
    (string=
     "
@@ -239,8 +321,7 @@ rule abc:
 rule abc:
     input: 'infile'
 <point>output:"
-      (snakemake-indent-line)
-      (snakemake-indent-line)
+      (snakemake-indent-line 'prev)
       (buffer-string))))
   (should
    (string=
@@ -270,10 +351,6 @@ rule abc:
       (buffer-substring (point-min) (point))))))
 
 (ert-deftest snakemake-test-indent-line/field-value ()
-  ;; Always indent line below naked field key to
-  ;; `snakemake-indent-field-offset' +
-  ;; `snakemake-indent-value-offset'.  Move point to to this position
-  ;; as well if it is before any text on the line.
   (should
    (string=
     "
@@ -298,8 +375,7 @@ rule abc:
 rule abc:
     output:
 <point>"
-      (snakemake-indent-line)
-      (snakemake-indent-line)
+      (snakemake-indent-line 'prev)
       (buffer-string))))
   (should
    (string=
@@ -314,24 +390,6 @@ rule abc:
               <point>"
       (snakemake-indent-line)
       (buffer-string))))
-
-  ;; Add step with Python indentation for non-blank lines under naked
-  ;; field keys.  Field keys with values starting on the same line do
-  ;; not use Python indentation because this is invalid syntax in
-  ;; Snakemake.
-  (should
-   (string=
-    "
-rule abc:
-    output: 'file{}{}'.format('one',
-    'two'"
-    (snakemake-with-temp-text
-        "
-rule abc:
-    output: 'file{}{}'.format('one',
-<point>'two'"
-      (snakemake-indent-line)
-      (buffer-string))))
   (should
    (string=
     "
@@ -351,39 +409,6 @@ rule abc:
    (string=
     "
 rule abc:
-    output:
-        'file{}{}'.format('one',
-    "
-    (snakemake-with-temp-text
-        "
-rule abc:
-    output:
-        'file{}{}'.format('one',
-<point>"
-      (snakemake-indent-line)
-      (buffer-string))))
-
-  ;; On non-naked field key cycle indentation between
-  ;; `snakemake-indent-field-offset' and column of previous field
-  ;; value.  If point is before any text on the line, move it to the
-  ;; start of the text instead.
-  (should
-   (string=
-    "
-rule abc:
-    output: 'file'
-    "
-    (snakemake-with-temp-text
-        "
-rule abc:
-    output: 'file'
-<point>"
-      (snakemake-indent-line)
-      (buffer-string))))
-  (should
-   (string=
-    "
-rule abc:
     output: 'file'
             "
     (snakemake-with-temp-text
@@ -392,7 +417,6 @@ rule abc:
     output: 'file'
 <point>"
       (snakemake-indent-line)
-      (snakemake-indent-line)
       (buffer-string))))
   (should
    (string=
@@ -404,23 +428,8 @@ rule abc:
         "
 rule abc:
     output: 'file'
-<point>"
-      (snakemake-indent-line)
-      (snakemake-indent-line)
-      (snakemake-indent-line)
-      (buffer-string))))
-  (should
-   (string=
-    "
-rule abc:
-    output: 'file'
-    'text'"
-    (snakemake-with-temp-text
-        "
-rule abc:
-    output: 'file'
-<point>'text'"
-      (snakemake-indent-line)
+             <point>"
+      (snakemake-indent-line 'prev)
       (buffer-string))))
   (should
    (string=
@@ -434,14 +443,13 @@ rule abc:
     output: 'file'
 <point>'text'"
       (snakemake-indent-line)
-      (snakemake-indent-line)
       (buffer-string))))
   (should
    (string=
     "
 rule abc:
     output: 'file'
-    'text' "
+            'text' "
     (snakemake-with-temp-text
         "
 rule abc:
@@ -454,7 +462,7 @@ rule abc:
     "
 rule abc:
     output: 'file'
-  "
+            "
     (snakemake-with-temp-text
         "
 rule abc:
@@ -472,12 +480,9 @@ rule abc:
         "
 rule abc:
     output: 'file'
-<point>  'text'"
-      (snakemake-indent-line)
-      (snakemake-indent-line)
+<point>            'text'"
+      (snakemake-indent-line 'prev)
       (buffer-string))))
-
-  ;; Indent body of run field according to Python mode.
   (should
    (string=
     "
@@ -511,6 +516,21 @@ output:"
   (should
    (string=
     "
+if True:
+    rule abc:
+        input: 'infile'
+        output:"
+    (snakemake-with-temp-text
+        "
+if True:
+<point>rule abc:
+input: 'infile'
+output:"
+      (indent-region (point) (point-max))
+      (buffer-string))))
+  (should
+   (string=
+    "
 rule abc:
     input:
         one='one', two='two'
@@ -613,115 +633,6 @@ subworkflow otherworkflow:
     snakefile: '../path/to/otherworkflow/Snakefile'"
     (should (snakemake-in-rule-or-subworkflow-block-p))))
 
-(ert-deftest snakemake-test-first-field-line-p ()
-  (snakemake-with-temp-text
-      "
-rule abc:
-<point>"
-    (should (snakemake-first-field-line-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-<point>    output: 'file'"
-    (should (snakemake-first-field-line-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-    output:
-<point>"
-    (should-not (snakemake-first-field-line-p))))
-
-(ert-deftest snakemake-test-below-naked-field-p ()
-  (snakemake-with-temp-text
-      "
-rule abc:
-    output:
-<point>"
-    (should (snakemake-below-naked-field-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-    output: 'file'
-<point>"
-    (should-not (snakemake-below-naked-field-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-    output: <point>"
-    (should-not (snakemake-below-naked-field-p))))
-
-(ert-deftest snakemake-test-naked-field-line-p ()
-  (snakemake-with-temp-text
-      "
-rule abc:
-    output:
-<point>"
-    (should (snakemake-naked-field-line-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-    output:
-        'file',
-         <point>"
-    (should (snakemake-naked-field-line-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-    output: <point>"
-    (should (snakemake-naked-field-line-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-    output: 'file'
-<point>"
-    (should-not (snakemake-naked-field-line-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-    input:
-        'infile'
-    output: 'file'
-<point>"
-    (should-not (snakemake-naked-field-line-p))))
-
-(ert-deftest snakemake-test-run-field-line-p ()
-  (snakemake-with-temp-text
-      "
-rule abc:
-    run:
-<point>"
-    (should (snakemake-run-field-line-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-    run:
-        with file:
-<point>"
-    (should (snakemake-run-field-line-p)))
-  (snakemake-with-temp-text
-      "
-rule abc:
-    output: 'file'
-<point>"
-    (should-not (snakemake-run-field-line-p))))
-
-(ert-deftest snakemake-test-previous-field-value-column ()
-  (should (= 12
-             (snakemake-with-temp-text
-                 "
-rule abc:
-    output: 'file'
-<point>"
-               (snakemake-previous-field-value-column))))
-  (should (= 12
-             (snakemake-with-temp-text
-                 "
-rule abc:
-    output: 'file',
-            'another'
-<point>"
-               (snakemake-previous-field-value-column)))))
-
 
 ;;; snakemake.el