Hi!

Thanks for your comments, that's exactly what I was hoping to get from someone who is familiar with the Lyx internals.

Attached is a fixed up version of the patch, mostly incorporating the points you raised, but see below for specifics.

Two major changes (well, as major as anything relating to this patch can be ;) ): 1) I do preserve font information now, so it works for multiple languages, too; however, this means that I transpose individual chars, not a string. 2) In change-tracking mode, I now ignore the deleted characters when determining which positions to transpose. Before this fix, performing two transposes in a row in change-tracking mode had weird results; now it works the same whether or not change-tracking is on.

Thanks again for your comments, I'd be happy to hear any more.
Dov

Jean-Marc Lasgouttes wrote:
This is rather amusing. The functionality has been removed (probably by
mistake) by Andre' and nobody noticed :) I guess this is the reason why
Michael wanted to remove it, but he did not tell us.


Well, at least now I know that I'm working on something really
important... ;) Seriously, though, I think *I'll* use it...

... I think it is better to exit early when the cursor is
at one end of the paragraph.

Done.

Handling insets would be very nice, but
it was not done in 1.3 either.

I'm not sure that there is much meaning to doing a transpose with insets... the main thing I ever use transpose for is for typos.

A few remarks/questions though: - why is transpose a char array and not a string?

My C++/STL is a little rusty; I couldn't find any way to create a string out of a single char... Anyhow, I'm not using either in the end, since the transposition is now done one char at a time, in order to accommodate font/language transitions.

- why do you use recordUndoSelection, whereas there is no selection?
  (BTW, I think the lfun should be enabled only when there is no
  selection).

I now use recordUndo. (What's the difference, though? I mean, it seemed to work before, too, so why is there a need for the two separate functions?)

- there is no need to define 'from'. Please use cur.pit() and
  cur.pos() directly.

Done.

- instead of using cur.paragraph() and pars_[par] (which are the same
  thing), you'd better define
        Paragraph & par = cur.paragraph();
  and use it throughout.

Done.

All in all I guess the function can be made shorter.

Well, it was until I added the "major changes" I mentioned above... ;)

...
The language is part of the font. You could refuse to transpose if the
font is not the same on both sides of the cursor.

Thanks, I've used that information now to perform the transposition correctly.

...
JMarc
Index: src/lyxtext.h
===================================================================
--- src/lyxtext.h       (revision 15158)
+++ src/lyxtext.h       (working copy)
@@ -244,6 +244,8 @@
        };
        /// Change the case of the word at cursor position.
        void changeCase(LCursor & cur, TextCase action);
+       /// Transpose characters at cursor position.
+       void charsTranspose(LCursor & cur);
 
        /** the DTP switches for paragraphs. LyX will store the top settings
         always in the first physical paragraph, the bottom settings in the
Index: src/text3.C
===================================================================
--- src/text3.C (revision 15158)
+++ src/text3.C (working copy)
@@ -728,7 +728,7 @@
                break;
 
        case LFUN_TRANSPOSE_CHARS:
-               recordUndo(cur);
+               charsTranspose(cur);
                break;
 
        case LFUN_PASTE:
Index: src/text.C
===================================================================
--- src/text.C  (revision 15158)
+++ src/text.C  (working copy)
@@ -1585,6 +1585,63 @@
 }
 
 
+// Transposes the character at the cursor with the one before it.
+void LyXText::charsTranspose(LCursor & cur)
+{
+       BOOST_ASSERT(this == cur.text());
+
+       pos_type pos = cur.pos();
+       
+       // If cursor is at beginning or end of paragraph, do nothing.
+       if (pos == cur.lastpos() || pos == 0)
+               return;
+       
+       Paragraph & par = cur.paragraph();
+
+       // Get the positions of the characters to be transposed. 
+       pos_type pos1 = pos - 1;
+       pos_type pos2 = pos;
+
+       // In change tracking mode, ignore deleted characters.
+       while (pos2 < cur.lastpos() && isDeletedText(par, pos2))
+               ++pos2;
+       if (pos2 == cur.lastpos())
+               return;
+               
+       while (pos1 >= 0 && isDeletedText(par, pos1))
+               --pos1;
+       if (pos1 < 0)
+               return;
+       
+       // Don't do anything if one of the "characters" is not regular text.
+       if (par.isInset(pos1) || par.isInset(pos2))
+               return;
+
+       // Store the characters to be transposed (including font information).
+       char char1 = par.getChar(pos1);
+       LyXFont const font1 = 
+               par.getFontSettings(cur.buffer().params(), pos1);
+       
+       char char2 = par.getChar(pos2);
+       LyXFont const font2 = 
+               par.getFontSettings(cur.buffer().params(), pos2);
+
+       // And finally, we are ready to perform the transposition.
+       // We use erase() and insertChar(), which take care of tracking the
+       // change.
+       recordUndo(cur);
+
+       par.erase(pos2);
+       par.erase(pos1);
+       par.insertChar(pos1, char2, font2);
+       par.insertChar(pos2, char1, font1);
+       
+       // After the transposition, move cursor to after the transposition.
+       setCursor(cur, cur.pit(), pos2);
+       cur.forwardPos();
+}
+
+
 bool LyXText::Delete(LCursor & cur)
 {
        BOOST_ASSERT(this == cur.text());

Reply via email to