emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Shift selection using interactive spec


From: Stefan Monnier
Subject: Re: Shift selection using interactive spec
Date: Sat, 22 Mar 2008 21:39:35 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux)

>> The way I see it "only-TTM" is a more brittle and less used&tested
>> implementation than "temporary-TTM", so everything else being equal
>> I prefer "temporary-TTM".  In this case I'm not convinced the difference
>> matters, so I prefer "temporary-TTM".

> Here's a patch that changes `only' mode so that it is not explicitly
> turned off by the command loop.  It should play nicely with
> transient-mark-mode, temporary mark (C-SPC C-SPC), and mouse
> selection.  Play around with the shift selection and let me know what
> you think.

> + Lisp_Object Qhandle_shift_selection;

I'm wondering if its name should hint at "movement-command" rather
than at "shift".

> ! If the string begins with `^' and `this-command-keys-shift-translated'
> !  is non-nil, Emacs calls `handle-shift-selection' before reading
> !  any arguments.
> ! You may use `@', `*', and `^' together; they are processed in the
> !  order that they appear.
>   usage: (interactive ARGS)  */)
>        (args)
>        Lisp_Object args;
> ***************
> *** 447,452 ****
> --- 453,463 ----
>           }
>         string++;
>       }
> +       else if (*string == '^')
> +     {
> +       call0 (Qhandle_shift_selection);
> +       string++;
> +     }
>         else break;
>       }

The code is right, the docstring is not.
  
> --- 1758,1775 ----
>       }
>         else
>       {
> !       if (NILP (current_kboard->Vprefix_arg)
> !           && NILP (Vthis_command_keys_shift_translated))
>           {
>             /* In case we jump to directly_done.  */
>             Vcurrent_prefix_arg = current_kboard->Vprefix_arg;
  
>             /* Recognize some common commands in common situations and
>                do them directly.  */
> !           if (EQ (Vthis_command, Qforward_char) && PT < ZV
> !               && NILP (Vthis_command_keys_shift_translated)
> !               && (NILP (Vtransient_mark_mode)
> !                   || EQ (Vtransient_mark_mode, Qt)))
>               {
>                     struct Lisp_Char_Table *dp
>                   = window_display_table (XWINDOW (selected_window));

[ Side remark, please ignore ]
Yuck!  We really should get rid of this "optimization".

> ***************
> *** 1961,1974 ****
  
>         if (!NILP (current_buffer->mark_active) && !NILP (Vrun_hooks))
>       {
> -       /* Setting transient-mark-mode to `only' is a way of
> -          turning it on for just one command.  */
> - 
> -       if (EQ (Vtransient_mark_mode, Qidentity))
> -         Vtransient_mark_mode = Qnil;
> -       if (EQ (Vtransient_mark_mode, Qonly))
> -         Vtransient_mark_mode = Qidentity;
> - 
>         if (!NILP (Vdeactivate_mark) && !NILP (Vtransient_mark_mode))
>           {
>             /* We could also call `deactivate'mark'.  */

This is good ;-)

> +   /* This is non-zero if we are trying to map a key by changing an
> +      upper-case letter to lower-case or a shifted function key to an
> +      unshifted one.  */
> +   volatile int shift_translated = 0;

We should either directly set Vthis_command_keys_shift_translated or
explain why we have this two-step approach where we first set
shift-translated and then assign it to Vthis_command_keys_shift_translated.

> ***************
> *** 3322,3327 ****
> --- 3322,3331 ----
>     (cond
>      ((eq transient-mark-mode 'lambda)
>       (setq transient-mark-mode nil))
> +    ((eq transient-mark-mode 'only)
> +     (setq transient-mark-mode nil))
> +    ((eq (car-safe transient-mark-mode) 'only)
> +     (setq transient-mark-mode (cdr transient-mark-mode)))
>      (transient-mark-mode
>       (setq mark-active nil)
>       (run-hooks 'deactivate-mark-hook))))

Yuck.  But I guess that's what we get for removing the corresponding C code.

> ***************
> *** 3487,3492 ****
> --- 3491,3499 ----
>       (if arg
>       (pop-to-mark-command)
>         (push-mark-command t)))
> +    ((eq (car-safe transient-mark-mode) 'only)
> +     (deactivate-mark)
> +     (push-mark-command nil))
>      ((and set-mark-command-repeat-pop
>        (eq last-command 'pop-to-mark-command))
>       (setq this-command 'pop-to-mark-command)

Even more yuck.

Here's an another solution: let's remove all the `only' thingy in
`transient-mark-mode', and instead we introduce a new variable for it,
which says whether the mark is highlighted for just this one command.

So we'd define

   (defun temporary-region-highlight ()
     (unless mark-active-just-once
       (setq mark-active-just-once t)
       (unless transient-mark-mode
         (setq transient-mark-mode 'lambda))
       (push-mark nil nil t)))
   
   (defun temporary-region-unhighlight ()
     (when mark-active-just-once
       (deactivate-mark)))

and of course deactivate-mark needs to set mark-active-just-once to nil.
I don't particularly like the name "mark-active-just-once" either.
   

        Stefan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]