>>>>> "Dov" == Dov Feldstern <[EMAIL PROTECTED]> writes:
Dov> While trying to get my bearings in the code, I noticed that there
Dov> was a stub for a transpose function (LFUN_TRANSPOSE_CHARS), so I
Dov> thought that writing that function would be a good exercise for
Dov> getting familiar with the code. Attached is a patch (against
Dov> 1.4.3) which adds the transpose functionality.
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.
You may want to take a look at the 1.3 implementation:
void LyXText::transposeChars(BufferView & bview)
{
Paragraph * tmppar = cursor.par();
setUndo(&bview, Undo::FINISH, tmppar, tmppar->next());
pos_type tmppos = cursor.pos();
// First decide if it is possible to transpose at all
// We are at the beginning of a paragraph.
if (tmppos == 0) return;
// We are at the end of a paragraph.
if (tmppos == tmppar->size() - 1) return;
unsigned char c1 = tmppar->getChar(tmppos);
unsigned char c2 = tmppar->getChar(tmppos - 1);
if (c1 != Paragraph::META_INSET
&& c2 != Paragraph::META_INSET) {
tmppar->setChar(tmppos, c2);
tmppar->setChar(tmppos - 1, c1);
}
// We should have an implementation that handles insets
// as well, but that will have to come later. (Lgb)
checkParagraph(const_cast<BufferView*>(&bview), tmppar, tmppos);
}
The inner data structure have changed, but it is probably
understandable. I think it is better to exit early when the cursor is
at one end of the paragraph. Handling insets would be very nice, but
it was not done in 1.3 either.
Other than that, your code looks very good and fits well with the rest
of the source. Congratulations!
A few remarks/questions though:
- why is transpose a char array and not a string?
- why do you use recordUndoSelection, whereas there is no selection?
(BTW, I think the lfun should be enabled only when there is no
selection).
- there is no need to define 'from'. Please use cur.pit() and
cur.pos() directly.
- 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.
All in all I guess the function can be made shorter.
Dov> (Of course, to make it at all useful, it really needs to be bound
Dov> to a key --- I bound it to C-t for myself, but I didn't include
Dov> that in the patch, as C-t is already taken.)
It is bound to C-t in emacs bindings.
Dov> A few comments about the patch: * It took me a while to figure
Dov> this out, but I was finally able to get it so that undo sees the
Dov> transposition as a single atomic action;
Dov> * the same goes for making sure that the transposition *is*
Dov> tracked by the change-tracking mechanism.
Very good.
Dov> * There is still a problem with a transposition which takes place
Dov> on the border between two languages (at least, I have the problem
Dov> when moving between English and Hebrew, which is RTL). There's no
Dov> real reason to ever want to transpose such a case, so the patch
Dov> could be committed as is; on the other hand, if someone can help
Dov> me out with fixing this, that would be great. (What I need to
Dov> know is, how do I identify the transition between
Dov> languages/directions? I'd just as soon just cancel the action if
Dov> I recognize that I'm on such a border.)
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.
Dov> * I have a feeling that what I currently do for checking that
Dov> both characters are "regular text" (and not an inset, for
Dov> example), is not complete; however, it seems to work. Again, if
Dov> someone could help me out with how better to check for this
Dov> condition, I'll fix the code if necessary.
I think your test is OK.
Dov> Is there any documentation for developers? Specifically,
Dov> something which explains in general how the document/buffer is
Dov> stored in memory (i.e.: how are insets, language changes, etc.
Dov> represented)?
Unfortunately not.
Dov> Finally, back to word-completion: I've started working on that,
Dov> but the main problem I've run into is preserving state between
Dov> consecutive invocations. For example, let's say C-p is bound to
Dov> this function. So the first time I call C-p, the word which the
Dov> cursor is at is completed. The problem is, if I immediately
Dov> (i.e., before moving the cursor or doing anything else in the
Dov> document) press C-p again, I would like to get the next possible
Dov> completion. But (a) how do I know that this is not the first call
Dov> to the completion function, and (b) is there any way to keep
Dov> track of the list of possible completion that I already found the
Dov> first time around, so that I don't need to go through the whole
Dov> buffer again?
This I do not know...
JMarc
