Hi Søren, On Thu, Sep 08, 2005 at 19:37:41 +0200, Søren Thing Pedersen wrote:
> Many thanks for your easy to follow instructions. I have done as > suggested. Patches attached. Comments are most welcome. Looks ok syntactically, just some nitpicking on style: > + bool bAssumeEnglishNumbers = TRUE; > + > + bAssumeEnglishNumbers = > SC_MOD()->GetAppOptions().GetNumberRecognitionAssumeEnglish() Why initialize first and then immediately assign another value? I'd write that as BOOL bAssumeEnglishNumbers = SC_MOD()->GetAppOptions().GetNumberRecognitionAssumeEnglish() Note also the use of bool/true versus BOOL/TRUE, strictly spoken they are different types. Semantically there's a slight difference between the naming of NumberRecognitionAssumeEnglish and what the patch respectively the already existing code actually does: it doesn't necessarily parse a number as an en-US number; it will still replace the decimal separator under certain conditions to convert from an en-US number, but will always parse a number of the current locale. It's more a NumberRecognitionFuzzyness ;-) In your case, as it is a personal change, it wouldn't be that important, but otherwise I'd say the configuration entry somehow should also indicate that it affects HTML and RTF only, either in the description or the naming, or both. In the main code line I would even split the code for HTML and RTF at that place and implement something different for RTF that takes the RTF language tag into account, which would be the proper solution. Eike -- OOo/SO Calc core developer. Number formatter bedevilled I18N transpositionizer. GnuPG key 0x293C05FD: 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
