bug#2421: 23.0.90; python-mode: Indent commands behavior

bug#2421: 23.0.90; python-mode: Indent commands behavior

[Milan Zamazal]

In GNU Emacs 23.0.90.1 (x86_64-pc-linux-gnu, GTK+ Version 2.12.11)
 of 2009-02-03 on blackbird, modified by Debian
 (emacs-snapshot package, version 1:20090202-1)

The indent commands `C-c <' and `C-c >' work only on active regions.
This is annoying, inconsistent with `C-x TAB' behavior, and not useful
(even if it worked as announced in its documentation, i.e. shifting the
current line, which it doesn't, a similar effect can be achieved more
easily by TAB presses).

So I suggest to change the behavior of these commands to always work on
the region, whether it is active or not, or to provide a configuration
option which can enable such a behavior.

Regards,

Milan Zamazal

bug#2421: 23.0.90; python-mode: Indent commands behavior

[martin rudalics]

 > The indent commands `C-c <' and `C-c >' work only on active regions.
 > This is annoying, inconsistent with `C-x TAB' behavior, and not useful
 > (even if it worked as announced in its documentation, i.e. shifting the
 > current line, which it doesn't,

I changed that yesterday, so it now should work as documented.

 > a similar effect can be achieved more
 > easily by TAB presses).
 >
 > So I suggest to change the behavior of these commands to always work on
 > the region, whether it is active or not, or to provide a configuration
 > option which can enable such a behavior.

Isn't that shift behavior something that should be done via rectangles?
IIRC, someone even proposed to keep the region active after the command
completes in order to apply a second shift step if needed.

martin

bug#2421: 23.0.90; python-mode: Indent commands behavior

[Milan Zamazal]

>>>>> "mr" == martin rudalics <rudalics@gmx.at> writes:

    mr> Isn't that shift behavior something that should be done via
    mr> rectangles?  

Do you mean by the user?  No, this would require marking the region on
exact characters rather than lines, i.e. it would be less comfortable
and would make the operation more visual than logical.  `C-x TAB' is
also not superfluous.

    mr> IIRC, someone even proposed to keep the region active after the
    mr> command completes in order to apply a second shift step if
    mr> needed.

This would be OK for me.

Regards,

Milan Zamazal

bug#2421: 23.0.90; python-mode: Indent commands behavior

[martin rudalics]

 >     mr> Isn't that shift behavior something that should be done via
 >     mr> rectangles?
 >
 > Do you mean by the user?  No, this would require marking the region on
 > exact characters rather than lines, i.e. it would be less comfortable

You're right that rect.el doesn't have suitable code for highlighting
and dragging rectangles.  cua-rect.el has two functions for shifting
rectangles, but I don't know whether they could be used here.

 > and would make the operation more visual than logical.

That was my intention.  I don't know whether `python-shift-left' and
`python-shift-right' are of any use without a prefix argument.  With a
prefix argument, they are simple wrappers for `indent-rigidly'.  And
`indent-rigidly' is a purely visual operation.  That is, if indenting
part of a program changes the semantics of the program, it's the user
who's responsible, without any "logical" support from `python-mode'.

But since I never use these functions you shouldn't take my remarks too
seriously.  Maybe you could propose a patch to provide the behavior you
like.  In any case, it might be better to use `use-region-p' instead of
`mark-active' to be more consistent with Emacs 23.

martin

bug#2421: 23.0.90; python-mode: Indent commands behavior

[Milan Zamazal]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

>>>>> "mr" == martin rudalics <rudalics@gmx.at> writes:

    mr> I don't know whether `python-shift-left' and
    mr> `python-shift-right' are of any use without a prefix argument.

They are, I often use them after adding or removing a statement in a
code.
    
    mr> With a prefix argument, they are simple wrappers for
    mr> `indent-rigidly'.  

Yes, this makes no sense.  It would be more logical if they indented by
(* python-indent count).

    mr> Maybe you could propose a patch to provide the behavior you
    mr> like.  

OK, see below for some suggestion.

    mr> In any case, it might be better to use `use-region-p' instead of
    mr> `mark-active' to be more consistent with Emacs 23.

But then the indent commands would work only if transient-mark-mode was
enabled, wouldn't they?

2009-02-23  Milan Zamazal  <pdm@zamazal.org>

	* python.el (python-shift-left, python-shift-right): When called
	with a prefix argument, indent by that many levels, not columns.

	* python.el (python-shift-left, python-shift-right): Don't
	deactivate the active mark.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: python.el.patch --]
[-- Type: text/x-diff, Size: 1747 bytes --]

--- python.el.orig	2009-02-23 19:06:19.000000000 +0100
+++ python.el	2009-02-23 19:18:37.000000000 +0100
@@ -2013,17 +2013,18 @@
        (list (region-beginning) (region-end) current-prefix-arg)
      (list (line-beginning-position) (line-end-position) current-prefix-arg)))
   (if count
-      (setq count (prefix-numeric-value count))
+      (setq count (* python-indent (prefix-numeric-value count)))
     (setq count python-indent))
-  (when (> count 0)
-    (save-excursion
-      (goto-char start)
-      (while (< (point) end)
-	(if (and (< (current-indentation) count)
-		 (not (looking-at "[ \t]*$")))
-	    (error "Can't shift all lines enough"))
-	(forward-line))
-      (indent-rigidly start end (- count)))))
+  (let (deactivate-mark)
+    (when (> count 0)
+      (save-excursion
+        (goto-char start)
+        (while (< (point) end)
+          (if (and (< (current-indentation) count)
+                   (not (looking-at "[ \t]*$")))
+              (error "Can't shift all lines enough"))
+          (forward-line))
+        (indent-rigidly start end (- count))))))
 
 (add-to-list 'debug-ignored-errors "^Can't shift all lines enough")
 
@@ -2036,10 +2037,11 @@
    (if mark-active
        (list (region-beginning) (region-end) current-prefix-arg)
      (list (line-beginning-position) (line-end-position) current-prefix-arg)))
-  (if count
-      (setq count (prefix-numeric-value count))
-    (setq count python-indent))
-  (indent-rigidly start end count))
+  (let (deactivate-mark)
+    (if count
+        (setq count (* python-indent (prefix-numeric-value count)))
+      (setq count python-indent))
+    (indent-rigidly start end count)))
 
 (defun python-outline-level ()
   "`outline-level' function for Python mode.

[-- Attachment #3: Type: text/plain, Size: 25 bytes --]


Regards,

Milan Zamazal

bug#2421: 23.0.90; python-mode: Indent commands behavior

[martin rudalics]

 >     mr> I don't know whether `python-shift-left' and
 >     mr> `python-shift-right' are of any use without a prefix argument.
 >
 > They are, I often use them after adding or removing a statement in a
 > code.

I'm confused.  Why don't you use TAB in that case?

 >     mr> With a prefix argument, they are simple wrappers for
 >     mr> `indent-rigidly'.
 >
 > Yes, this makes no sense.  It would be more logical if they indented by
 > (* python-indent count).

It might be even more logical to restrict ourselves to valid indentation
points only.  In any case we would need some feedback from `python-mode'
users for such a change.  The same goes for not deactivating the mark.

 >     mr> In any case, it might be better to use `use-region-p' instead of
 >     mr> `mark-active' to be more consistent with Emacs 23.
 >
 > But then the indent commands would work only if transient-mark-mode was
 > enabled, wouldn't they?

The commands should DTRT when `transient-mark-mode' is enabled because
it's the default with Emacs 23.  That is, with `transient-mark-mode' on,
these commands should operate on the region only if the mark is active
and should respect `use-empty-active-region'.  I have no idea what TRT
is now when `transient-mark-mode' is off.  You appear reluctant to use
transient mark mode, I suppose ...

martin

bug#2421: 23.0.90; python-mode: Indent commands behavior

[Milan Zamazal]

>>>>> "mr" == martin rudalics <rudalics@gmx.at> writes:

    mr> Why don't you use TAB in that case?

Because I didn't know it can work on regions!  Perhaps this makes the
python-shift-* commands redundant for my use, I'll see.  I'm sorry for
the confusion.

Regards,

Milan Zamazal

bug#2421: 23.0.90; python-mode: Indent commands behavior

[Milan Zamazal]

>>>>> "mr" == martin rudalics <rudalics@gmx.at> writes:

    mr> I don't know whether `python-shift-left' and
    mr> `python-shift-right' are of any use without a prefix argument.

    >> 
    >> They are, I often use them after adding or removing a statement
    >> in a code.

    mr> I'm confused.  Why don't you use TAB in that case?

Well, actually TAB runs indent-region.  But I need simple shift instead.
Consider the following example:

  def foo():
      for i in range(3):
  print i
  print i + 1
  print i + 2

I just pasted the print lines from a differently indented code and want
to fix the indentation by shifting the region of the print lines:

  def foo():
      for i in range(3):
          print i
          print i + 1
          print i + 2

TAB can't do it:

  def foo():
      for i in range(3):
          print i
  print i + 1
  print i + 2

Neither repeated pressing of C->:

  def foo():
      for i in range(3):
      print i
      print i + 1
      print i + 2

To get what I want I have to press `C-8 C->'.  I don't like it, I'd like
to call `M-x SOMETHING-shift-right' twice or
`M-2 M-x SOMETHING-shift-right'.

    >> It would be more logical if they indented by (* python-indent
    >> count).

    mr> It might be even more logical to restrict ourselves to valid
    mr> indentation points only.

Perhaps, but it's not that important.

    mr> In any case we would need some feedback from `python-mode' users
    mr> for such a change.  The same goes for not deactivating the mark.

If people like the current behavior, configuration options may be
introduced.

    mr> I have no idea what TRT is now when `transient-mark-mode' is
    mr> off.  

Neither do I.

Regards,

Milan Zamazal

bug#2421: 23.0.90; python-mode: Indent commands behavior

[martin rudalics]

 > Well, actually TAB runs indent-region.  But I need simple shift instead.
 > Consider the following example:
 >
 >   def foo():
 >       for i in range(3):
 >   print i
 >   print i + 1
 >   print i + 2
 >
 > I just pasted the print lines from a differently indented code and want
 > to fix the indentation by shifting the region of the print lines:
 >
 >   def foo():
 >       for i in range(3):
 >           print i
 >           print i + 1
 >           print i + 2
 >
 > TAB can't do it:
 >
 >   def foo():
 >       for i in range(3):
 >           print i
 >   print i + 1
 >   print i + 2

We could pass the prefix argument to `indent-region' with the semantics
that the latter ignores the mode-specific `indent-region-function' and
calls `indent-line' for each line in the region instead.

Alternatively, `python-indent-region' could, like `python-indent-line',
remember whether the last command was an `indent-for-tab-command' and
call `python-indent-line-1' with no argument for each line in the
region.  Obviously, the region would have to stay to make this
practical.  And in some pathological cases it might be hard to find the
appropriate indentation points.

Anyhow, I'm afraid that people won't approve such changes.

 > To get what I want I have to press `C-8 C->'.  I don't like it, I'd like
 > to call `M-x SOMETHING-shift-right' twice or
 > `M-2 M-x SOMETHING-shift-right'.

That's why I earlier said that some rectangle code should handle this.

martin

bug#2421: 23.0.90; python-mode: Indent commands behavior

[Milan Zamazal]

Hm, it seems the easiest solution will be that I put my own definitions
to ~/.emacs.

Regards,

Milan Zamazal

bug#2421: 23.0.90; python-mode: Indent commands behavior

[Fabián Ezequiel Gallina]

Trunk's python mode works the same whether the region is active or not.

Closing this one.