Review Request: autocorrect config file lookup change

[Laurent Montel]

> On Nov. 6, 2012, 6:04 p.m., Laurent Montel wrote:
> > plugins/textediting/autocorrection/Autocorrect.cpp, line 707
> > <http://git.reviewboard.kde.org/r/107229/diff/1/?file=93886#file93886line707>
> >
> >     You never breaks here ?
> >     So you will read a lot of file no ?
> 
> C. Boemann wrote:
>     yes that is the point - I want to first read the general kde wide file and then the calligra specific file.
>     
>     I could also revert your original patch and just read the calligra one
>     
>     the point is we need af file with the calligra specifics
> 
> Laurent Montel wrote:
>     "The special tags in the calligra supplied config files where never found" which special tags?
>     
>     global file and calligra file is the same (installed from l10n) So What is the problem ?
> 
> C. Boemann wrote:
>     Uhm it's not the same file. It may be so in kde 4.9 onwards or so but we should be compatible with kde 4.5
>     
>     but calligra needs special tags that I for one don't have in my .kde/share/apps/autocorrect/* file
> 
> Laurent Montel wrote:
>     I don't understand because it's installed from same file from l10n...
>     So what is missing tag ?
> 
> C. Boemann wrote:
>     for example:
>     
>     
>      <SuperScript>
>       <superscript find="1st" super="st" />
>       <superscript find="2nd" super="nd" />
>       <superscript find="3rd" super="rd" />
>       <superscript find="othernb" super="th" />
>      </SuperScript>
>
> 
> Laurent Montel wrote:
>     Error is not that file changed, it's that we don't store it.
>     When I looked at calligra code there was not save code => you didn't save it.
>     => I added it, and missing to add this part.
>     
>     
>     I will create a patch for it.
>     (it's right I don't use it in kmail => I didn't see that it was missing)
>     
>     Will fix it soon.
>     
>     When you added this plugins to calligra (don't know who) rewrite it and forgot to add write code...
> 
> Laurent Montel wrote:
>     I fixed save superscript in kmail and calligra
>     regards
> 
> C. Boemann wrote:
>     Ok thanks for that, but it still leaves the problem that it's not loaded in the first place
>     
>     There are more things here:
>     
>      1) the folders << "/"
>         at the very least I think it should be "" For me the found filename was "/autocorrect/en_US.xml" which is a file that doesn't even exist, yet findResource returns as if it did. This may be due to a bug/difference in older versions of findResource.
>     
>      2) even if I fix the above then the file I do load "/home/cbo/.kde/share/apps/autocorrect/autocorrect.xml" doesn't contain the superscript part. This is probably a derivative problem of us not saving in the past, but it also means that the file is still around and will be loaded.
>
> 
> Laurent Montel wrote:
>     1) don't know but if it fixes for you loading change to ""
>     2) we can't real fix it without remove this "/home/cbo/.kde/share/apps/autocorrect/autocorrect.xml" in the past we didn't save it so it's not a real problem to remove it and recreate it locally now that save/load is ok.
>     (I just added saving few weeks ago so it will not impact a lot of calligra release I think).
> 
> C. Boemann wrote:
>     1) ok
>     2) ok that is good news
>     
>     But! we have a bigger problem too: If both apps save to "/home/cbo/.kde/share/apps/autocorrect/autocorrect.xml" then we have a problem if the set of tags doesn't correspond. what if in the future we want to ad another tag, then if the user still uses an old kmail too, then the xml file will be overwritten and the setting lost.
>     
>     Also if we ever want to add a new set of tags it will never be loaded because "/home/cbo/.kde/share/apps/autocorrect/autocorrect.xml" will take precedence

Autocorrect.xml is for all kde so it's right if in calligra or kmail we add new tag it will not saved/loaded.
But by default we mustn't add new tag without ask to kdepim or calligra dev :)
For the moment we can't shared save/load code but perhaps for the future we can move it in kdelibs for example to fix it.

Perhaps we can add version in xml file, but I think we will look at it when calligra dev or kdepim dev (me :) ) will want to add more tag.
Not necessary for the moment to increase complexity of code I think.


- Laurent


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107229/#review21494
-----------------------------------------------------------


On Nov. 6, 2012, 5:54 p.m., C. Boemann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107229/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2012, 5:54 p.m.)
> 
> 
> Review request for Calligra and Laurent Montel.
> 
> 
> Description
> -------
> 
> The special tags in the calligra supplied config files where never found
> 
> So I've changed it so we first look up the general files and then the calligra specific files on top
> 
> A problem I see is that the general contents is never really used with my approach if we afterwards find a calligra special file
> 
> So a better suggestion is appreciated
> 
> 
> Diffs
> -----
> 
>   plugins/textediting/autocorrection/Autocorrect.h 5b76121 
>   plugins/textediting/autocorrection/Autocorrect.cpp 53fdee6 
> 
> Diff: http://git.reviewboard.kde.org/r/107229/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> C. Boemann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20121107/cb88bf19/attachment.htm>