bug#16438: 24.3.50; `C-x TAB right right' fails with error (mark-inactive)

bug#16438: 24.3.50; `C-x TAB right right' fails with error (mark-inactive)

[Juri Linkov]

1. emacs -Q
2. Paste to *scratch*:

(custom-set-variables
 '(mark-even-if-inactive nil))

3. Eval it.
4. Put point at its opening paren and type:

C-M-SPC    - selects the whole expression
C-x TAB    - activates a transient indentation mode
right      - adjusts indentation 1 position to the right
right      - fails with the error "The mark is not active now"
             because the previous `right' key deactivated the region

One way to fix this is not to deactivate the mark in this transient mode:

=== modified file 'lisp/indent.el'
--- lisp/indent.el	2014-01-13 05:03:31 +0000
+++ lisp/indent.el	2014-01-14 08:07:55 +0000
@@ -214,7 +214,9 @@ (defun indent-rigidly (start end arg &op
               (indent-to (max 0 (+ indent (prefix-numeric-value arg))) 0))
           (delete-region (point) (progn (skip-chars-forward " \t") (point))))
         (forward-line 1))
-      (move-marker end nil))))
+      (move-marker end nil)
+      (when (eq overriding-terminal-local-map indent-rigidly-map)
+	(setq deactivate-mark nil)))))


But the condition of checking for the transient mode doesn't work
because overriding-terminal-local-map never equals to indent-rigidly-map.

 indent-rigidly-map:
  (keymap (S-right . indent-rigidly-right-to-tab-stop) (S-left . indent-rigidly-left-to-tab-stop) (right . indent-rigidly-right) (left . indent-rigidly-left))

 overriding-terminal-local-map:
  (keymap (keymap (S-right . indent-rigidly-right-to-tab-stop) (S-left . indent-rigidly-left-to-tab-stop) (right . indent-rigidly-right) (left . indent-rigidly-left))
          add-keymap-witness)

There is some `add-keymap-witness' at the end of `overriding-terminal-local-map'
when the transient indentation mode is active.  And I don't know another way
to check if `overriding-terminal-local-map' is active.

bug#16438: 24.3.50; `C-x TAB right right' fails with error (mark-inactive)

[Juri Linkov]

> But the condition of checking for the transient mode doesn't work
> because overriding-terminal-local-map never equals to indent-rigidly-map.
>
>  indent-rigidly-map:
>   (keymap (S-right . indent-rigidly-right-to-tab-stop) (S-left . indent-rigidly-left-to-tab-stop) (right . indent-rigidly-right) (left . indent-rigidly-left))
>
>  overriding-terminal-local-map:
>   (keymap (keymap (S-right . indent-rigidly-right-to-tab-stop) (S-left . indent-rigidly-left-to-tab-stop) (right . indent-rigidly-right) (left . indent-rigidly-left))
>           add-keymap-witness)

I fixed this by using (eq (cadr overriding-terminal-local-map) indent-rigidly-map)

bug#16438: 24.3.50; `C-x TAB right right' fails with error (mark-inactive)

[Stefan Monnier]

> I fixed this by using (eq (cadr overriding-terminal-local-map) indent-rigidly-map)

That's hideous, tho :-(


        Stefan

bug#16438: 24.3.50; `C-x TAB right right' fails with error (mark-inactive)

[Juri Linkov]

>> I fixed this by using (eq (cadr overriding-terminal-local-map) indent-rigidly-map)
>
> That's hideous, tho :-(

I borrowed this condition from

    (not (eq map (cadr overriding-terminal-local-map)))

in `set-transient-map' that checks if the transient map is in effect.

Maybe it should be refactored to a separate predicate function.

bug#16438: 24.3.50; `C-x TAB right right' fails with error (mark-inactive)

[Stefan Monnier]

>>> I fixed this by using (eq (cadr overriding-terminal-local-map) indent-rigidly-map)
>> That's hideous, tho :-(
> I borrowed this condition from
>     (not (eq map (cadr overriding-terminal-local-map)))

I know.  But it has problems:
- if there's a nested use of set-transient-map (e.g. C-u), your code will
  deactivate the mark even tho we're not finished with C-x TAB.
- it relies on internal details of implementation of set-transient-map.
- now C-x TAB does not deactivate the mark any more :-(

> Maybe it should be refactored to a separate predicate function.

Could be, tho the two tests need to be different.
Another option is to move the deactivation into the exit condition.
Yet another option is to change C-x TAB so that it uses (mark t) when
called repeatedly.

I'm not sure exactly how the deactivation should behave, tho:
- The current behavior seems wrong: C-x TAB does use the region, so it
  should "consume it", so C-x TAB right right right up should end with
  a deactivated region.
- But should the region be highlighted *during* C-x TAB?  If yes, then
  we have a conflict with the previous point, since just before hitting
  `up' the region would still need to be highlighted, so it'd be counter
  intuitive to deactivate the region right between when the user hits
  `up' and when we run the corresponding `previous-line' command.

So I think that the region should *not* be highlighted during C-x TAB.
Which implies using (mark t).


        Stefan

bug#16438: 24.3.50; `C-x TAB right right' fails with error (mark-inactive)

[Juri Linkov]

> - But should the region be highlighted *during* C-x TAB?

When the user selects a region, this indicates the intention
to run the next command on the selected region.  So when
a command is going to operate on the region the user should have
the clear indication beforehand to avoid surprises.

Highlighting the region in transient indentation mode is
especially important since this is a new feature,
and without highlighting an unsuspecting user don't know that
a subsequent `right' will operate on the previously selected region.

For the same reason I think the exit condition should not
deactivate the active region because the highlighting
indicates that the next command is operating on the region.

The same problem with exiting the transient indentation mode
happens when the user needs to type the `right' key to move point
without indentation.  Then the user has to exit the sub-mode
explicitly by typing e.g. C-g.  It will also deselect the region.

bug#16438: 24.3.50; `C-x TAB right right' fails with error (mark-inactive)

[Stefan Monnier]

> For the same reason I think the exit condition should not
> deactivate the active region because the highlighting
> indicates that the next command is operating on the region.

Indeed, that's the problem with keeping the highlighting, since, this
means that C-x TAB will not "consume" the region, which is unusual and
inconvenient: after C-x TAB right right right, the user will often have
to use C-g to deactivate the region :-(


        Stefan