corrections for transpose-chars

[Daniel Clemente-2]

Hi.

1. I don't understand why C-t (transpose-chars) won't swap two-letter fields, e.g. og → go. The first patch below makes this work. Note that other programs (readline, bash, Emacs) do the same.

2. Can transpose-chars be also activated for the minibuffer? The second patch does this. I feel I should have used minibuffer-transpose-chars instead of transpose-chars, just like the other keys do, but I don't know why the minibuffer- prefix was used. In fact, where are all those functions like minibuffer-backward-word defined? Could you add some explanatory comments?

Thanks

-- Daniel

[1]:

diff --git a/modules/commands.js b/modules/commands.js
index 6e77a22..1ecced2 100644
--- a/modules/commands.js
+++ b/modules/commands.js
@@ -125,10 +125,9 @@ function transpose_chars (field) {
     var caret = field.selectionStart; // Caret position.
     var length = value.length;
 
-    // If we have less than two character in the field or if we are at the
-    // beginning of the field, do nothing.
-    if (length <= 2 || caret == 0)
-        return;
+    // If we are at the beginning of the field, do nothing.
+ if (caret==0)
+ return;
 
     // If we are at the end of the field, switch places on the two last
     // characters. TODO: This should happen at the end of every line, not only




[2]:

diff --git a/modules/bindings/default/minibuffer.js b/modules/bindings/default/minibuffer.js
index 969afcb..8f0f27a 100644
--- a/modules/bindings/default/minibuffer.js
+++ b/modules/bindings/default/minibuffer.js
@@ -23,6 +23,7 @@ define_key(minibuffer_base_keymap, "C-d", "minibuffer-cmd_deleteCharForward");
 define_key(minibuffer_base_keymap, "delete", "minibuffer-cmd_deleteCharForward");
 define_key(minibuffer_base_keymap, "M-d", "minibuffer-cmd_deleteWordForward");
 define_key(minibuffer_base_keymap, "C-delete", "minibuffer-cmd_deleteWordForward");
+define_key(minibuffer_base_keymap, "C-t", "transpose-chars");
 define_key(minibuffer_base_keymap, "C-b", "minibuffer-backward-char");
 define_key(minibuffer_base_keymap, "left", "minibuffer-backward-char");
 define_key(minibuffer_base_keymap, "M-b", "minibuffer-backward-word");

_______________________________________________
Conkeror mailing list
Conkeror@...
https://www.mozdev.org/mailman/listinfo/conkeror

[Deniz Dogan-3]

Hi, Daniel

Thank you for the patches!

2009/9/24 Daniel Clemente <dcl441-bugs@...>:
> Hi.
>
> 1. I don't understand why C-t (transpose-chars) won't swap two-letter fields, e.g. og → go. The first patch below makes this work. Note that other programs (readline, bash, Emacs) do the same.

The problem with my initial version (I wrote the original function)
was that it checked for "length <= 2" when it should have checked for
"length < 2". The rationale is that there is no point in trying to
transpose characters on a line with zero or one character. I can't
test your patch, because I don't have Conkeror readily available at
the moment.

> 2. Can transpose-chars be also activated for the minibuffer? The second patch does this. I feel I should have used minibuffer-transpose-chars instead of transpose-chars, just like the other keys do, but I don't know why the minibuffer- prefix was used. In fact, where are all those functions like minibuffer-backward-word defined? Could you add some explanatory comments?

Yes, it should be activated in the minibuffer as well, but I don't
know why the "minibuffer-" prefix on commands exists. A wild guess is
that maybe M-<backspace> should echo "Text is read-only" (like in
Emacs) which doesn't happen in e.g. web text fields.

--
Deniz Dogan
_______________________________________________
Conkeror mailing list
Conkeror@...
https://www.mozdev.org/mailman/listinfo/conkeror

[Jeremy Maitin-Shepard-2]

"deniz.a.m.dogan" <deniz.a.m.dogan@...> writes:

> Hi, Daniel
> Thank you for the patches!

> 2009/9/24 Daniel Clemente <dcl441-bugs@...>:
>> Hi.
>>
>> 1. I don't understand why C-t (transpose-chars) won't swap two-letter fields,
> e.g. og → go. The first patch below makes this work. Note that other programs
> (readline, bash, Emacs) do the same.

> The problem with my initial version (I wrote the original function)
> was that it checked for "length <= 2" when it should have checked for
> "length < 2". The rationale is that there is no point in trying to
> transpose characters on a line with zero or one character. I can't
> test your patch, because I don't have Conkeror readily available at
> the moment.

>> 2. Can transpose-chars be also activated for the minibuffer? The second patch
> does this. I feel I should have used minibuffer-transpose-chars instead of
> transpose-chars, just like the other keys do, but I don't know why the
> minibuffer- prefix was used. In fact, where are all those functions like
> minibuffer-backward-word defined? Could you add some explanatory comments?

> Yes, it should be activated in the minibuffer as well, but I don't
> know why the "minibuffer-" prefix on commands exists. A wild guess is
> that maybe M-<backspace> should echo "Text is read-only" (like in
> Emacs) which doesn't happen in e.g. web text fields.

Commands that operate on the minibuffer are separate from the commands
that operate on the buffer.  Due to architectural differences between
Mozilla (and Conkeror) and Emacs, this allows things to be more robust
to focus issues.

--
Jeremy Maitin-Shepard
_______________________________________________
Conkeror mailing list
Conkeror@...
https://www.mozdev.org/mailman/listinfo/conkeror

[Daniel Clemente-2]

> The rationale is that there is no point in trying to
> transpose characters on a line with zero or one character.
  I forgot the case length==1, but it seems to work well. Add if(length>1) if needed.


> Yes, it should be activated in the minibuffer as well, but I don't
> know why the "minibuffer-" prefix on commands exists.

  Taking an example:
define_key(minibuffer_base_keymap, "M-f", "minibuffer-forward-word");

  Still harder is to know where a command named minibuffer-forward-word was defined. I couldn't find any with grep, and also there are no signs of the string "minibuffer-" being added to function names.
  Therefore I think that a comment could explain the magic behind this.


-- Daniel

_______________________________________________
Conkeror mailing list
Conkeror@...
https://www.mozdev.org/mailman/listinfo/conkeror

[John J. Foerch]

On Mon, Sep 28, 2009 at 08:33:29AM +0200, Daniel Clemente wrote:

>
> > The rationale is that there is no point in trying to
> > transpose characters on a line with zero or one character.
>   I forgot the case length==1, but it seems to work well. Add if(length>1) if needed.
>
>
> > Yes, it should be activated in the minibuffer as well, but I don't
> > know why the "minibuffer-" prefix on commands exists.
>
>   Taking an example:
> define_key(minibuffer_base_keymap, "M-f", "minibuffer-forward-word");
>
>   Still harder is to know where a command named minibuffer-forward-word was defined. I couldn't find any with grep, and also there are no signs of the string "minibuffer-" being added to function names.
>   Therefore I think that a comment could explain the magic behind this.

Grep for define_builtin_commands.. incidentally this needs to be
rewritten, but since it works "good enough" it's not at the top of my
to-do.

--
John Foerch
_______________________________________________
Conkeror mailing list
Conkeror@...
https://www.mozdev.org/mailman/listinfo/conkeror

[Daniel Clemente-2]

El dl, set 28 2009 a les 17:51, John J. Foerch va escriure:
> Grep for define_builtin_commands.. incidentally this needs to be
> rewritten, but since it works "good enough" it's not at the top of my
> to-do.

Could you check this patch in?


diff --git a/modules/bindings/default/minibuffer.js b/modules/bindings/default/minibuffer.js
index 465d4f8..077f743 100644
--- a/modules/bindings/default/minibuffer.js
+++ b/modules/bindings/default/minibuffer.js
@@ -28,6 +28,7 @@ define_key(minibuffer_base_keymap, "C-d", "minibuffer-cmd_deleteCharForward");
 define_key(minibuffer_base_keymap, "delete", "minibuffer-cmd_deleteCharForward");
 define_key(minibuffer_base_keymap, "M-d", "minibuffer-cmd_deleteWordForward");
 define_key(minibuffer_base_keymap, "C-delete", "minibuffer-cmd_deleteWordForward");
+define_key(minibuffer_base_keymap, "C-t", "transpose-chars");
 define_key(minibuffer_base_keymap, "C-b", "minibuffer-backward-char");
 define_key(minibuffer_base_keymap, "left", "minibuffer-backward-char");
 define_key(minibuffer_base_keymap, "M-b", "minibuffer-backward-word");
diff --git a/modules/commands.js b/modules/commands.js
index 699f497..e9bb33e 100644
--- a/modules/commands.js
+++ b/modules/commands.js
@@ -128,7 +128,7 @@ function transpose_chars (field) {
 
     // If we have less than two character in the field or if we are at the
     // beginning of the field, do nothing.
-    if (length <= 2 || caret == 0)
+    if (length < 2 || caret == 0)
         return;
 
     // If we are at the end of the field, switch places on the two last



Thanks

_______________________________________________
Conkeror mailing list
Conkeror@...
https://www.mozdev.org/mailman/listinfo/conkeror

[John J. Foerch]

On Fri, Oct 09, 2009 at 09:21:14AM +0200, Daniel Clemente wrote:
> El dl, set 28 2009 a les 17:51, John J. Foerch va escriure:
> > Grep for define_builtin_commands.. incidentally this needs to be
> > rewritten, but since it works "good enough" it's not at the top of my
> > to-do.
>
> Could you check this patch in?
>


Thanks.  pushed.

--
John Foerch

_______________________________________________
Conkeror mailing list
Conkeror@...
https://www.mozdev.org/mailman/listinfo/conkeror