Re: [PUSHED] Re: [PATCH 2/2] Smarter auto-complete capitalization (#i22961#) and i18n handling

2012-06-06 Thread Caolán McNamara
On Wed, 2012-06-06 at 21:53 +1200, Brad Sowden wrote:
> I just checked and your test case passes when sTest = "Some text" but if 
> I change it to sTest = "Some tExt" it fails (assuming "Some Text" is the 
> right result).

That looks sort of dubious alright. Way too many classes and wrappers
here. Its unclear to me whether that characterclassifcation thing is
intended to just operate on characters and convert all of them to a
given case, in which case its broken for title case since
https://issues.apache.org/ooo/show_bug.cgi?id=30863 or whether its
really supposed to do what it does now, which doesn't make a lot of
sense either.

> The following function appears to do what I originally wanted (i.e. 
> Title case the first character and Lower case subsequent characters) but 
> using opengrok I can't find anywhere that actually instantiates this class.
> 
> rtl::OUString SAL_CALL Transliteration_titlecase::transliterate()

Looks like TransliterationWrapper in unotools is the "route" to that,
e.g. utl::TransliterationWrapper aTrans(factory,
TransliterationModulesExtra::TITLE_CASE);

where the call to the ctor for Transliteration_titlecase is hidden away
inside TransliterationImpl::loadModuleByName. Putting a breakpoint on
Transliteration_titlecase::Transliteration_titlecase and using the
"format->change case->Capitalize Every Word" in writer gives the bt for
it.

Its all the usual overengineered horror :-)

> http://opengrok.libreoffice.org/xref/core/i18npool/source/transliteration/transliteration_body.cxx#329
> 
> Anyway, I think the solution I used in my patch is better as I actually 
> wanted Sentence case rather than Title case for the overall string.

fwiw that utl::TransliterationWrapper has a sentence case mode.

C.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PUSHED] Re: [PATCH 2/2] Smarter auto-complete capitalization (#i22961#) and i18n handling

2012-06-06 Thread Brad Sowden

Hi Caolán,


* Sidenote - Initially I tried using CharClass::titlecase() but
discovered this doesn't actually work.


What did you try and what went wrong ?


There appears to be an issue with
class cclass_Unicode where "trans" is only ever set to
"Transliteration_casemapping" and there is no mechanism to set it to
"Transliteration_titlecase".


I believe that the casemapping thing there knows about upper, lower and
title case and uses setMappingType to MappingTypeToTitle to toggle which
one to return

e.g. I added this test
http://cgit.freedesktop.org/libreoffice/core/commit/?id=eb9b8ebca49291797e655b50f64af2c2fa03434c
to confirm to myself that its working like I think its supposed to.


When I called CharClass::titlecase() I expected "ORAN" to be changed to 
"Oran" but from memory the returned string was still "ORAN". Attached is 
the back trace I took at the time.


Looking again in cclass_Unicode::toTitle()

http://opengrok.libreoffice.org/xref/core/i18npool/source/characterclassification/cclass_unicode.cxx#92

I see only the first character in each word is Title cased and the rest 
are left untouched. Oh!


105 for (sal_Int32 i = nPos; i < nCount + nPos; i++, out++) {
106 if (i >= bdy.endPos)
107 bdy = brk.nextWord(Text, bdy.endPos, rLocale,
108 WordType::ANYWORD_IGNOREWHITESPACES);
109 *out = (i == bdy.startPos) ?
110 trans->transliterateChar2Char(Text[i]) : Text[i];
111 }

I just checked and your test case passes when sTest = "Some text" but if 
I change it to sTest = "Some tExt" it fails (assuming "Some Text" is the 
right result).


The following function appears to do what I originally wanted (i.e. 
Title case the first character and Lower case subsequent characters) but 
using opengrok I can't find anywhere that actually instantiates this class.


rtl::OUString SAL_CALL Transliteration_titlecase::transliterate()

http://opengrok.libreoffice.org/xref/core/i18npool/source/transliteration/transliteration_body.cxx#329

Anyway, I think the solution I used in my patch is better as I actually 
wanted Sentence case rather than Title case for the overall string.


Cheers,
Brad
#0  com::sun::star::i18n::casefolding::getValue (str=0x7fffaa9c, pos=0, len=1, aLocale=...,
nMappingType=16 '\020') at /home/brad/git/libo/i18nutil/source/utility/casefolding.cxx:94
#1  0x7fffe5a02b03 in com::sun::star::i18n::Transliteration_body::transliterateChar2Char (
this=, inChar=79)
at /home/brad/git/libo/i18npool/source/transliteration/transliteration_body.cxx:215
#2  0x7fffe59c3cf3 in com::sun::star::i18n::cclass_Unicode::toTitle (this=0x1d0d240,
Text="ORAN", nPos=, nCount=, rLocale=...)
at /home/brad/git/libo/i18npool/source/characterclassification/cclass_unicode.cxx:110
#3  0x7fffe59c7b6f in com::sun::star::i18n::CharacterClassificationImpl::toTitle (
this=, Text="ORAN", nPos=0, nCount=, rLocale=)
at /home/brad/git/libo/i18npool/source/characterclassification/characterclassificationImpl.cxx:74
#4  0x74d5ff4b in CharClass::titlecase (this=, rStr="ORAN", nPos=0,
nCount=4) at /home/brad/git/libo/unotools/source/i18n/charclass.cxx:284
#5  0x7fffe1dc5c31 in titlecase (_rStr="ORAN", this=0x1d0b2a0)
at /home/brad/git/libo/solver/unxlngx6.pro/inc/unotools/charclass.hxx:187
#6  QuickHelpData::FillStrArr (this=0x160fce0, rSh=..., rWord="ORAN")
at /home/brad/git/libo/sw/source/ui/docvw/edtwin.cxx:5589
#7  0x7fffe1dc7040 in SwEditWin::ShowAutoTextCorrectQuickHelp (this=,
rWord="ORAN", pACfg=0x7fffe0e7c320, pACorr=0x15feab0, bFromIME=0 '\000')
at /home/brad/git/libo/sw/source/ui/docvw/edtwin.cxx:5701___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PUSHED] Re: [PATCH 2/2] Smarter auto-complete capitalization (#i22961#) and i18n handling

2012-06-05 Thread Caolán McNamara
On Mon, 2012-06-04 at 22:44 +0200, Michael Stahl wrote:
> On 04/06/12 14:03, Brad Sowden wrote:
> > * Sidenote - Initially I tried using CharClass::titlecase() but 
> > discovered this doesn't actually work. 

What did you try and what went wrong ?

> > There appears to be an issue with 
> > class cclass_Unicode where "trans" is only ever set to 
> > "Transliteration_casemapping" and there is no mechanism to set it to 
> > "Transliteration_titlecase".

I believe that the casemapping thing there knows about upper, lower and
title case and uses setMappingType to MappingTypeToTitle to toggle which
one to return

e.g. I added this test
http://cgit.freedesktop.org/libreoffice/core/commit/?id=eb9b8ebca49291797e655b50f64af2c2fa03434c
to confirm to myself that its working like I think its supposed to.

C.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


[PUSHED] Re: [PATCH 2/2] Smarter auto-complete capitalization (#i22961#) and i18n handling

2012-06-04 Thread Michael Stahl
On 04/06/12 14:03, Brad Sowden wrote:
> Hi,
> 
> This patch implements the following:
> 
> (1) Completes a TODO in the code related to i18n handling of month and 
> day names with auto-completion. Previously, when non-ASCII names were 
> added to the auto-complete word list these had to be an exact 
> case-sensitive match of the word to be auto-completed. This patch allows 
> a case-insensitive match for non-ASCII names when deciding what names to 
> add to the auto-complete list.
> 
> (2) Smarter auto-complete capitalization as described in OpenOffice bug 
> 22961. If a word is in sentence case then the auto-completed word should 
> be in the same case i.e. if the auto-complete list contains the word 
> "LIBRE" then "Lib" should auto-complete to "Libre" rather
> than "LibRE".
> 
> https://issues.apache.org/ooo/show_bug.cgi?id=22961

hi Brad,

thanks a lot for the patch, that is a nice improvement (2 improvements
even), i've pushed it to master.

> * Sidenote - Initially I tried using CharClass::titlecase() but 
> discovered this doesn't actually work. There appears to be an issue with 
> class cclass_Unicode where "trans" is only ever set to 
> "Transliteration_casemapping" and there is no mechanism to set it to 
> "Transliteration_titlecase".

hmmm... don't know much about that unfortunately...

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice