[PATCH 1/2] Replace SvStringsISortDtor in edtwin.cxx and gloslst.[ch]xx

Hi,

This patch converts a SvStringsISortDtor to a vector and maintains
equivalent functionality.

Instead of sorting strings upon each insertion all strings are now added
and then a single sort and "unique" are applied (SvStringsISortDtor
behaves like a SET with regards to insertion, which is why "unique" is
applied). SwEditWin::ShowAutoTextCorrectQuickHelp() is the only place
where strings are added to the vector.

The change in behaviour from keeping the first inserted version of a
string to keeping the first version post-sort (case-insensitive ASCII
comparison) should have no material impact as the strings retrieved from
SwAutoCompleteWord are already unique (case-insensitive ASCII
comparison) and the capitalization of the string is generally changed
anyway to match the capitalization of the word to be auto-completed.
Also, there appears to be no logical reason to store the first inserted
version of a string over the first version post-sort.

* In previous patches that I've seen on the list it appears ok to
convert a vector of String pointers to simply a vector of String values
so I've done the same.
* I used a pointer to the vector as the move() function previously
copied and cleared all elements whereas a pointer swap would suffice.
* I've put a TODO in the code to replace the ASCII only case-insensitive
sort (existing limitation).

- make + make dev-install successful
- functionality tested ok
- licence statement on file

Cheers,
Brad


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

On 04/06/12 14:01, Brad Sowden wrote:
> Hi,
>
> This patch converts a SvStringsISortDtor to a vector and maintains
> equivalent functionality.

hi Brad,

thanks for the patch, i've pushed it to master.

hmmm... makes sense.

> * In previous patches that I've seen on the list it appears ok to
> convert a vector of String pointers to simply a vector of String values
> so I've done the same.

yes that's a nice improvement.

> * I used a pointer to the vector as the move() function previously
> copied and cleared all elements whereas a pointer swap would suffice.

ah that's why, i had wondered....
would it be possible to use std::swap on the vector itself rather than
the pointer?   i could imagine that being implemented efficiently, and
it would allow the vector to not be a pointer, which is simpler.

also (but that was a pre-existing condition, not really your fault)
member names should start with "m_" so it's easy to grep for member access.

> * I've put a TODO in the code to replace the ASCII only case-insensitive
> sort (existing limitation).
>
> - make + make dev-install successful

that's a good start, please try to use "make check", which is a one-stop
command that does everything.  it probably wouldn't have found a problem
in this case, as i doubt there's a good test for autotext, but in
general it's a good idea.

> - functionality tested ok
> - licence statement on file

regards,
 michael

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Hi Michael,
Ah, I can use vector::swap(). Patch attached getting rid of the vector
pointer. Much nicer.

Thanks,
Brad

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

On 06/06/12 11:42, Brad Sowden wrote:
thanks, pushed :)
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice