python.el patch proposal: Respect `prog-indentation-context'.

python.el patch proposal: Respect `prog-indentation-context'.

[Wedler, Christoph]

May I push the following change?

-- Christoph

e579d8af825fab7b1844bc28908c2149891f9de6 HEAD master
Author: Christoph Wedler <christoph.wedler@sap.com>
Date:   Fri Jun 19 13:38:24 2015 +0000

    Respect `prog-indentation-context'.
    
    * lisp/progmodes/python.el (python-indent-guess-indent-offset)
    (python-indent-context, python-indent--calculate-indentation)
    (python-info-current-defun)
    (python-info-dedenter-opening-block-message)
    (python-info-line-ends-backslash-p)
    (python-info-beginning-of-backslash)
    (python-info-continuation-line-p): Use `prog-widen'.
    (python-indent--calculate-indentation)
    (python-indent--calculate-levels)
    (python-indent-calculate-indentation): Use `prog-first-column'.
    (python-indent--calculate-levels): Simplify.
    
    Ignore also initial empty lines for syntax calculation.
    * lisp/progmodes/python.el (python-indent-context): Return
    :no-indent for first non-empty line, not just in line 1.
    * test/automated/python-tests.el (python-indent-base-case)
    (python-indent-inside-paren-1, python-indent-inside-paren-2)
    (python-indent-inside-paren-3, python-indent-inside-paren-4)
    (python-indent-inside-paren-5, python-indent-inside-paren-6)
    (python-indent-after-backslash-1)
    (python-indent-after-backslash-2)
    (python-indent-after-backslash-3)
    (python-indent-after-backslash-4, python-indent-inside-string-1):
    Expect :no-indent for first non-empty line.

2 files changed, 41 insertions(+), 45 deletions(-)
 lisp/progmodes/python.el       | 60 ++++++++++++++++++++----------------------
 test/automated/python-tests.el | 26 +++++++++---------

	Modified   lisp/progmodes/python.el
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d45d082..f641880 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -734,7 +734,7 @@ work on `python-indent-calculate-indentation' instead."
   (interactive)
   (save-excursion
     (save-restriction
-      (widen)
+      (prog-widen)
       (goto-char (point-min))
       (let ((block-end))
         (while (and (not block-end)
@@ -833,7 +833,7 @@ keyword
  - Point is on a line starting a dedenter block.
  - START is the position where the dedenter block starts."
   (save-restriction
-    (widen)
+    (prog-widen)
     (let ((ppss (save-excursion
                   (beginning-of-line)
                   (syntax-ppss))))
@@ -958,18 +958,20 @@ keyword
        ((save-excursion
           (back-to-indentation)
           (skip-chars-backward " \t\n")
-          (python-nav-beginning-of-statement)
-          (cons
-           (cond ((python-info-current-line-comment-p)
-                  :after-comment)
-                 ((save-excursion
-                    (goto-char (line-end-position))
-                    (python-util-forward-comment -1)
-                    (python-nav-beginning-of-statement)
-                    (looking-at (python-rx block-ender)))
-                  :after-block-end)
-                 (t :after-line))
-           (point))))))))
+          (if (bobp)
+              (cons :no-indent 0)
+            (python-nav-beginning-of-statement)
+            (cons
+             (cond ((python-info-current-line-comment-p)
+                    :after-comment)
+                   ((save-excursion
+                      (goto-char (line-end-position))
+                      (python-util-forward-comment -1)
+                      (python-nav-beginning-of-statement)
+                      (looking-at (python-rx block-ender)))
+                    :after-block-end)
+                   (t :after-line))
+             (point)))))))))
 
 (defun python-indent--calculate-indentation ()
   "Internal implementation of `python-indent-calculate-indentation'.
@@ -978,10 +980,10 @@ current context or a list of integers.  The latter case is only
 happening for :at-dedenter-block-start context since the
 possibilities can be narrowed to specific indentation points."
   (save-restriction
-    (widen)
+    (prog-widen)
     (save-excursion
       (pcase (python-indent-context)
-        (`(:no-indent . ,_) 0)
+        (`(:no-indent . ,_) (prog-first-column)) ; usually 0
         (`(,(or :after-line
                 :after-comment
                 :inside-string
@@ -1019,7 +1021,7 @@ possibilities can be narrowed to specific indentation points."
          (let ((opening-block-start-points
                 (python-info-dedenter-opening-block-positions)))
            (if (not opening-block-start-points)
-               0  ; if not found default to first column
+               (prog-first-column) ; if not found default to first column
              (mapcar (lambda (pos)
                        (save-excursion
                          (goto-char pos)
@@ -1037,15 +1039,9 @@ integers.  Levels are returned in ascending order, and in the
 case INDENTATION is a list, this order is enforced."
   (if (listp indentation)
       (sort (copy-sequence indentation) #'<)
-    (let* ((remainder (% indentation python-indent-offset))
-           (steps (/ (- indentation remainder) python-indent-offset))
-           (levels (mapcar (lambda (step)
-                             (* python-indent-offset step))
-                           (number-sequence steps 0 -1))))
-      (reverse
-       (if (not (zerop remainder))
-           (cons indentation levels)
-         levels)))))
+    (nconc (number-sequence (prog-first-column) (1- indentation)
+                            python-indent-offset)
+           (list indentation))))
 
 (defun python-indent--previous-level (levels indentation)
   "Return previous level from LEVELS relative to INDENTATION."
@@ -1068,7 +1064,7 @@ minimum."
         (python-indent--previous-level levels (current-indentation))
       (if levels
           (apply #'max levels)
-        0))))
+        (prog-first-column)))))
 
 (defun python-indent-line (&optional previous)
   "Internal implementation of `python-indent-line-function'.
@@ -4230,7 +4226,7 @@ Optional argument INCLUDE-TYPE indicates to include the type of the defun.
 This function can be used as the value of `add-log-current-defun-function'
 since it returns nil if point is not inside a defun."
   (save-restriction
-    (widen)
+    (prog-widen)
     (save-excursion
       (end-of-line 1)
       (let ((names)
@@ -4413,7 +4409,7 @@ likely an invalid python file."
   (let ((point (python-info-dedenter-opening-block-position)))
     (when point
       (save-restriction
-        (widen)
+        (prog-widen)
         (message "Closes %s" (save-excursion
                                (goto-char point)
                                (buffer-substring
@@ -4434,7 +4430,7 @@ statement."
 With optional argument LINE-NUMBER, check that line instead."
   (save-excursion
     (save-restriction
-      (widen)
+      (prog-widen)
       (when line-number
         (python-util-goto-line line-number))
       (while (and (not (eobp))
@@ -4450,7 +4446,7 @@ With optional argument LINE-NUMBER, check that line instead."
 Optional argument LINE-NUMBER forces the line number to check against."
   (save-excursion
     (save-restriction
-      (widen)
+      (prog-widen)
       (when line-number
         (python-util-goto-line line-number))
       (when (python-info-line-ends-backslash-p)
@@ -4467,7 +4463,7 @@ When current line is continuation of another return the point
 where the continued line ends."
   (save-excursion
     (save-restriction
-      (widen)
+      (prog-widen)
       (let* ((context-type (progn
                              (back-to-indentation)
                              (python-syntax-context-type)))
	Modified   test/automated/python-tests.el
diff --git a/test/automated/python-tests.el b/test/automated/python-tests.el
index ae4323b..4585e7f 100644
--- a/test/automated/python-tests.el
+++ b/test/automated/python-tests.el
@@ -267,10 +267,10 @@ foo = long_function_name(
    (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (forward-line 1)
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (forward-line 1)
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))))
 
 (ert-deftest python-indent-after-comment-1 ()
@@ -392,7 +392,7 @@ data = {
 }
 "
    (python-tests-look-at "data = {")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (python-tests-look-at "'key':")
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
@@ -455,7 +455,7 @@ data = {'key': {
 }}
 "
    (python-tests-look-at "data = {")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (python-tests-look-at "'objlist': [")
    (should (eq (car (python-indent-context)) :inside-paren-newline-start))
@@ -494,7 +494,7 @@ data = ('these',
         'tokens')
 "
    (python-tests-look-at "data = ('these',")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (forward-line 1)
    (should (eq (car (python-indent-context)) :inside-paren))
@@ -514,7 +514,7 @@ data = [ [ 'these', 'are'],
          ['the', 'tokens' ] ]
 "
    (python-tests-look-at "data = [ [ 'these', 'are'],")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (forward-line 1)
    (should (eq (car (python-indent-context)) :inside-paren))
@@ -530,7 +530,7 @@ while ((not some_condition) and
         with_some_arg)
 "
    (python-tests-look-at "while ((not some_condition) and")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (forward-line 1)
    (should (eq (car (python-indent-context)) :inside-paren))
@@ -551,7 +551,7 @@ CHOICES = (('some', 'choice'),
            ('more', 'choices'))
 "
    (python-tests-look-at "CHOICES = (('some', 'choice'),")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (forward-line 1)
    (should (eq (car (python-indent-context)) :inside-paren))
@@ -612,7 +612,7 @@ from foo.bar.baz import something, something_1 \\\\
     something_4, something_5
 "
    (python-tests-look-at "from foo.bar.baz import something, something_1")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (python-tests-look-at "something_2 something_3,")
    (should (eq (car (python-indent-context)) :after-backslash-first-line))
@@ -639,7 +639,7 @@ objects = Thing.objects.all() \\\\
                        .values_list()
 "
    (python-tests-look-at "objects = Thing.objects.all()")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (python-tests-look-at ".filter(")
    (should (eq (car (python-indent-context))
@@ -682,7 +682,7 @@ with open('/path/to/some/file/you/want/to/read') as file_1, \\\\
 "
    (python-tests-look-at
     "with open('/path/to/some/file/you/want/to/read') as file_1, \\\\")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (python-tests-look-at
     "open('/path/to/some/file/being/written', 'w') as file_2")
@@ -703,7 +703,7 @@ super_awful_assignment = some_calculation() and \\\\
 "
    (python-tests-look-at
     "super_awful_assignment = some_calculation() and \\\\")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (python-tests-look-at "another_calculation() and \\\\")
    (should (eq (car (python-indent-context))
@@ -992,7 +992,7 @@ lines
 '''
 "
    (python-tests-look-at "multiline = '''")
-   (should (eq (car (python-indent-context)) :after-line))
+   (should (eq (car (python-indent-context)) :no-indent))
    (should (= (python-indent-calculate-indentation) 0))
    (python-tests-look-at "bunch")
    (should (eq (car (python-indent-context)) :inside-string))

Re: python.el patch proposal: Respect `prog-indentation-context'.

[Tom Tromey]

>>>>> "Christoph" == Wedler, Christoph <christoph.wedler@sap.com> writes:

Hi.

I'm supportive of your project.  I think it is great to see some
advances in this area in the core.

Christoph>    (save-restriction
Christoph> -    (widen)
Christoph> +    (prog-widen)
Christoph>      (let ((ppss (save-excursion
Christoph>                    (beginning-of-line)
Christoph>                    (syntax-ppss))))

I'm curious how you plan to extend your approach to handle syntax-ppss
as well.  It seems to me that the sub-mode has to be able to change the
syntax table at the mode boundaries; and furthermore that this also has
to be done during font-lock so that syntactic font-locking works
correctly in all the parts of the buffer.

If you already discussed this and I missed it, let me know and I can go
search the archives.  I don't remember seeing any mention of it, so I
thought I'd ask.

Tom

RE: python.el patch proposal: Respect `prog-indentation-context'.

[Wedler, Christoph]

> I'm supportive of your project.  I think it is great to see some
 > advances in this area in the core.

Thanks.  I have to admit, my change is just one step of many...

Christoph>    (save-restriction
Christoph> -    (widen)
Christoph> +    (prog-widen)
Christoph>      (let ((ppss (save-excursion
Christoph>                    (beginning-of-line)
Christoph>                    (syntax-ppss))))

 > I'm curious how you plan to extend your approach to handle
 > syntax-ppss as well.  It seems to me that the sub-mode has to be able
 > to change the syntax table at the mode boundaries

At the moment (in my private version of antlr-mode, which will be
release when I could have pushed my proposed change), I just dynamically
bind `syntax-ppss-cache' and `syntax-ppss-last' around the call of
`python-indent-line'.  Later, I will probably also set the syntax table
and related variables.  It would be excellent if every prog-mode would
have a <prog>-init-syntax-variables function which I could call for that
purpose.

In reality, the outer mode cannot support an inner mode with a complete
different (lexer) syntax (without care by the users). For example, I
have the following comment

;;  * Some constructs of languages (in actions) which are highly un-C-ish might
;;    bring Emacs (and probably ANTLR) out of sync: e.g. regexp literals in
;;    Perl, character and percent literals in Ruby.

...and I can remove the "probably", see the ANTLR3 distribution,
ANTLR3.g, rule NESTED_ACTION: the ANTLR tool just counts the braces
outside C-ish string/char literals and comments - so it is questionable
whether to really support a complete different syntax table or even
fancy things like `syntax-propertize-function'

 > and furthermore that this also has to be done during font-lock so that
 > syntactic font-locking works correctly in all the parts of the buffer.

Quite surprisingly, the syntax highlighting already looks quite good
without any special care for syntax tables (just
js--font-lock-keywords-3 were not nice).

But you are right, one should aim for improvements here, too.  At the
moment, it has lesser priority for me - after all, wrong syntax
highlighting just corrupts the appearance of the code whereas wrong
indentation corrupts the code itself.

I still update the antlr-mode to have all its feature available for
ANTLR v3 and v4 (options and makefile dependencies are still missing).
Then quite some cleanups are necessary (I still does not use syntax-ppss
in the mode itself as this feature was not available when the mode was
written).

- Christoph

Re: python.el patch proposal: Respect `prog-indentation-context'.

[Tom Tromey]

>>>>> "Christoph" == Wedler, Christoph <christoph.wedler@sap.com> writes:

Christoph> But you are right, one should aim for improvements here, too.  At the
Christoph> moment, it has lesser priority for me - after all, wrong syntax
Christoph> highlighting just corrupts the appearance of the code whereas wrong
Christoph> indentation corrupts the code itself.

I asked because it seems to me that many mode indenters look at the
syntax to decide what to do.

While digging around I found this in syntax.el:

    ;;; Todo:

    ;; - do something about the case where the syntax-table is changed.

So I suppose it is just a known problem.

The basic idea, I think, would be to let the mode(s) change the syntax
table when needed; and to have these changes be recorded by syntax.el so
that syntax-ppss and friends could use the correct syntax table for a
given region.

Tom

Re: python.el patch proposal: Respect `prog-indentation-context'.

[Stefan Monnier]

> The basic idea, I think, would be to let the mode(s) change the syntax
> table when needed; and to have these changes be recorded by syntax.el so
> that syntax-ppss and friends could use the correct syntax table for a
> given region.

Right, AFAIK, the way syntax-ppss works, it should already be possible
to make it work right without any changes to submodes.

It would just require changes in syntax.el or having the outer mode
install pieces of advice on some of the syntax.el functions.


        Stefan

RE: python.el patch proposal: Respect `prog-indentation-context'.

[Wedler, Christoph]

Christoph> But you are right, one should aim for improvements here, too.  At the
Christoph> moment, it has lesser priority for me - after all, wrong syntax
Christoph> highlighting just corrupts the appearance of the code whereas wrong
Christoph> indentation corrupts the code itself.

> I asked because it seems to me that many mode indenters look at the
> syntax to decide what to do.

You are right - I put "setting the syntax table of the inner mode" on my
TODO list as well.

> While digging around I found this in syntax.el:
>
>    ;;; Todo:
>    ;; - do something about the case where the syntax-table is changed.

As I bind syntax-ppss-cache and syntax-ppss-last to nil around the inner
mode indentation, this might already work - at least if I do not set the
inner mode's syntax-propertize function... (there are probably not too
many Python triple quotes strings in grammar actions...)

- Christoph

RE: python.el patch proposal: Respect `prog-indentation-context'.

[Wedler, Christoph]

> May I push the following change?

 > 2 files changed, 41 insertions(+), 45 deletions(-)
 >  lisp/progmodes/python.el       | 60 ++++++++++++++++++++----------------------
 >  test/automated/python-tests.el | 26 +++++++++---------

As I have heard no objections, I'll push the change this weekend.