Re: [Libreoffice] [PUSHED] Re: [PATCH 02/12] RTL_CONSTASCII_USTRINGPARAM in components cui options
Hi Wol, On 2010-11-23 at 20:49 +, Wols Lists wrote: String aFoo = String::createFromAscii( Something ) should be replaced, with String aFoo( RTL_CONSTASCII_USTRINGPARAM( Something ) ) Fine. Fixes in base will appear as I spot them :-) Just one important little point - the reason I asked is that I think the strings might actually be being passed to a non-OOo library - will this break a third-party library? (My C++-foo isn't good enough to answer this question for myself :-) This 'String' is an LibO internal class too (usually called tools string), so should be no problem. Or do you mean something different? Regards, Kendy ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PUSHED] Re: [PATCH 02/12] RTL_CONSTASCII_USTRINGPARAM in components cui options
Hi Wol, Kevin, all, On 2010-11-21 at 23:48 -0500, Kevin Hunter wrote: Forgive what might be a stupid question, but I've seen String::createFromAscii Will version 2 find those, and should they be replaced? Not a stupid question at all. Regular expressions aren't the most transparent of creatures. As I wrote the regex, round 2 /will/ find those. Since I don't know if those should be replaced, I assume that they shouldn't be, making this an edge case. The correct procedure then would be a Replace and Find as opposed to Replace All. This way one inspects every change rather than blindly updating every occurrence. Yes, even String::createFromAscii() usage should be replaced the similar way, the UniString class (that is the class aliased as String) has the appropriate constructor, ie. String aFoo = String::createFromAscii( Something ) should be replaced, with String aFoo( RTL_CONSTASCII_USTRINGPARAM( Something ) ) Regards, Kendy ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PUSHED] Re: [PATCH 02/12] RTL_CONSTASCII_USTRINGPARAM in components cui options
On 23/11/10 19:00, Jan Holesovsky wrote: Hi Wol, Kevin, all, On 2010-11-21 at 23:48 -0500, Kevin Hunter wrote: Forgive what might be a stupid question, but I've seen String::createFromAscii Will version 2 find those, and should they be replaced? Not a stupid question at all. Regular expressions aren't the most transparent of creatures. As I wrote the regex, round 2 /will/ find those. Since I don't know if those should be replaced, I assume that they shouldn't be, making this an edge case. The correct procedure then would be a Replace and Find as opposed to Replace All. This way one inspects every change rather than blindly updating every occurrence. Yes, even String::createFromAscii() usage should be replaced the similar way, the UniString class (that is the class aliased as String) has the appropriate constructor, ie. String aFoo = String::createFromAscii( Something ) should be replaced, with String aFoo( RTL_CONSTASCII_USTRINGPARAM( Something ) ) Regards, Kendy Fine. Fixes in base will appear as I spot them :-) Just one important little point - the reason I asked is that I think the strings might actually be being passed to a non-OOo library - will this break a third-party library? (My C++-foo isn't good enough to answer this question for myself :-) Cheers, Wol ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PUSHED] Re: [PATCH 02/12] RTL_CONSTASCII_USTRINGPARAM in components cui options
On 18/11/10 13:36, Kevin Hunter wrote: As I assume you're using a regex, you might consider catching this by doing the search and replace in series. Here's an example: 1. Catch the 'OUString +?= ...createFromAscii...' case and replace with 'OUString var( RTL...)' search: OUString\s*\w+\s*\+?=\s*\S*createFromAscii\(\s*([^]*)\s*\) replace: $1 $2( RTL_CONSTASCII_USTRINGPARAM( $4 )) 2. Then go back for a second pass with something like this: search: ::createFromAscii\(\s*([^]*)\s*\) replace: $1 $2( RTL_CONSTASCII_USTRINGPARAM( $4 )) The solution isn't perfect, as it still misses certain edge cases, but should at least help a little bit. Forgive what might be a stupid question, but I've seen String::createFromAscii Will version 2 find those, and should they be replaced? Cheers, Wol ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PUSHED] Re: [PATCH 02/12] RTL_CONSTASCII_USTRINGPARAM in components cui options
At 4:58pm -0500 Sun, 21 Nov 2010, Wols Lists wrote: On 18/11/10 13:36, Kevin Hunter wrote: As I assume you're using a regex, you might consider catching this by doing the search and replace in series. Here's an example: 1. Catch the 'OUString +?= ...createFromAscii...' case and replace with 'OUString var( RTL...)' search: OUString\s*\w+\s*\+?=\s*\S*createFromAscii\(\s*([^]*)\s*\) replace: $1 $2( RTL_CONSTASCII_USTRINGPARAM( $4 )) 2. Then go back for a second pass with something like this: search: ::createFromAscii\(\s*([^]*)\s*\) replace: $1 $2( RTL_CONSTASCII_USTRINGPARAM( $4 )) The solution isn't perfect, as it still misses certain edge cases, but should at least help a little bit. Forgive what might be a stupid question, but I've seen String::createFromAscii Will version 2 find those, and should they be replaced? Not a stupid question at all. Regular expressions aren't the most transparent of creatures. As I wrote the regex, round 2 /will/ find those. Since I don't know if those should be replaced, I assume that they shouldn't be, making this an edge case. The correct procedure then would be a Replace and Find as opposed to Replace All. This way one inspects every change rather than blindly updating every occurrence. Thanks for asking! Kevin P.S. As an aside, I'll make a minor plug that learning regular expressions makes possible or less mundane *many* tasks in the programming or text processing world, and many editors have regular expression support built-in. Vim, Emacs, jEdit, and Kate (among others) have them by default, while gEdit (among others) has them available via a plugin. If one doesn't know about them, one might be interested in perusing http://www.regular-expressions.info/, or just generically googling. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PUSHED] Re: [PATCH 02/12] RTL_CONSTASCII_USTRINGPARAM in components cui options
As I assume you're using a regex, you might consider catching this by doing the search and replace in series. Here's an example: 1. Catch the 'OUString +?= ...createFromAscii...' case and replace with 'OUString var( RTL...)' search: OUString\s*\w+\s*\+?=\s*\S*createFromAscii\(\s*([^]*)\s*\) replace: $1 $2( RTL_CONSTASCII_USTRINGPARAM( $4 )) 2. Then go back for a second pass with something like this: search: ::createFromAscii\(\s*([^]*)\s*\) replace: $1 $2( RTL_CONSTASCII_USTRINGPARAM( $4 )) The solution isn't perfect, as it still misses certain edge cases, but should at least help a little bit. Cheers, Kevin At 3:51pm -0500 Wed, 17 Nov 2010, Pierre-André Jacquod wrote: Sharp eyes.. Just to keep you trainded..:-( No really sorry, Despite reviewing diff, I did not catch this one. Will take more care On 11/17/2010 05:18 PM, Caolán McNamara wrote: On Tue, 2010-11-16 at 22:39 +0100, Pierre-André Jacquod wrote: On 11/16/2010 10:37 PM, Pierre-André Jacquod wrote: Hello, being off for some days, here the collection of patches I produced in between. Mostly good, but careful here, see... -aAutoStr += ::rtl::OUString::createFromAscii( ( ); +aAutoStr += ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(() ); you changed the string by accident from a bracket with a preceding space to one with no preceding space, clearly what's between has to remain the same :-). Fixed that typo and the rest looks good, pushed. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
