Am 20.07.2012, 20:00 Uhr, schrieb Jan Kundrát <[email protected]>:
it looks like my changes are conflicting, so patches 2-5 won't apply
i'll do a rebase on the comments then.
This is QStringBuilder, right?
Yes.
AFAIK you cannot use it in Qt 4.6
"In 4.6, an internal template class QStringBuilder has been added along
with a few helper functions. This class is marked internal and does not
appear in the documentation, because you aren't meant to instantiate it in
your code. Its use will be automatic, as described" [1] ... not here ;-)
The documentation may be wrong, but unless your Qt 4.6 users say "doesn't
work" i'd say "does" =)
+ - @p max defines the demanded maximum reply length - the return
can be longer, but the client will not make use of that
+ negative values mean "uncapped"
What is this used for?
Performance and/or remote queries. I don't consider typing "a" and getting
a list with a hundred entries which i will then navigate for the address a
use case - so there's no point in presenting more than <place random
number here> entries (or look them up, or transmit them)
This also falls into the "wanna use an AbstractModel instead" case.
+ // TODO: convince Jan to have more than one recp. per line
What for?
Convenience in writing recp. lists - i do see your "wonky parsing" point
though.
-> "wanna use an AbstractModel instead"?
+ struct Completion {
+ int pos, length;
+ QMenu *popup;
+ QLineEdit *receiver;
+ } m_completion;
I don't know what is this class for. Could you please add comments?
Just a container for some completion related variables.
Useless if we use a regular QCompleter
I try to have the headers sorted in a certain way -- see the docs [1].
25 #include "Sample.h"
26 #include "AnotherClass.h"
27 #include "BClass.h"
I assume "Sample.h" is the source header and "AnotherClass.h" and
"BClass.h" are random local includes?
(Otherwise if this is A : B in AnotherClass.cpp, that does not really mean
a trailing source header with trailing inheritage but no baseclass
included in the header, does it??)
+ m_addressBook = new AbookAddressbook;
Object ownership?
I placed it where the autocompletion used to be and because ..
because they are tied to the config options.
... i assume the addressbook backend should be configurable.
Well, haven't tried that patch as it doesn't apply, but I suspect it's
there for a reason. Where are you adding the extra widgets for new
addresses?
In the present layout.
The present code (not introduced by me) inserts them anyway behind a
certain index and this here was done only when removing an entry.
ratio
a) used to setFocus on optionally focus change (triggeres
editingFinished)
-> great way to annoy users (enter address, focus to subject -> focus
at new To)
b) if the user actually wanted to add a new address, he would not know
hot to until pressing
enter or set away the focus (see a ...)
c) old way was far too complex and allowed blank fields in the middle
-> new solution is to just ensure there's always a blank fiels at the
end
Okay, makes sense. Better to have 0003 and 0004 together, then (IMHO) --
They do rely on each other - the re-layouting was pointless ;-)
(I however would suggest to replace the OFFSET_OF_FIRST_ADDRESSEE based
injection with a sublayout because right now just removing the warning
would break things.
If it's racy, I don't like it. I guess that if a sync API makes is
easier for you, I can live with that.
The only actual danger is to change an item under the cursor while the
user clicks
Last note: if you want to keep the multientry lineedit, it's easy to
replace the popup by a combobox droppdown in case you just don't like
that
Not sure what this refers to, sorry -- been a long day here. Could you
please rephrase?
You'd if you had run the patches - the completer is a QMenu
But since you want to drop comma separation and likely don't care about
remote addressbook queries, one can as good use a model with fixed
matching function.
An incomplete set of requirements:
- it's possible to add/remove any recipients in any order
check
- it doesn't crash
hopefully - you know, that Turing guy said ... ;-)
- it isn't more hacky than the current version
check and see above for double check
- focus order is OK (=better than the current version)
will do (would have anyway)
- it doesn't leave empty fields behind
check
Cheers,
Thomas
[1] http://qt-project.org/doc/qt-4.8/qstring.html / you'll have to search
for the first "%"
