Re: Access2Base - New release

[Noel Power]

On 19/05/14 08:23, Stephan Bergmann wrote:
> On 05/16/2014 06:39 PM, Lionel Elie Mamane wrote:
>>
[...]
>> So, the question is "why does this code enforce this condition, and
>> can we change it"? Can we just remove the condition altogether, or
>> should we add this case:
>>
>>  || sOriginalUrl.match("$(INST)")
>>
>> Noel? Uray? You are our Basic "FindTheExpert"s. What's your opinion on
>> this?
So I wasn't following this, I was away for the last week anyway and
while reverse scanning my mail I stumbled on this. It seems to me that
the code there (which I admit I am not familiar with) is all to do with
extensions and management of extensions right? In this case you are
talking about trying to override a built-in library with an extension,
the code it would seem rightly tries to prevent an extension from doing
that. I mean there are wizards, conversions, routines etc. that are
considered part of the system that shouldn't be 'replaced' under the
hood.  Access2Base is considered a part of the core isn't it? it isn't
shipped as an extention, it is shipped as part of the product, it seems
to me that you are trying to get around the rules of no-new features
etc. by exploiting the extension mechanism. Access2Base is either part
of the product or it's not. Also doesn't the code mentioned above
actually try and remove the existing library? perhaps the
librarycontainer does something special in this case, I don't know.
But in anycase although Access2Base is part of the core, part of the
product etc. it is afaik completely selfcontained (and essentially a
separately maintained subsystem) in this case I think there is a good
argument to bend the rules regarding updating the version of Access2Base
shipped, we already do that occasionly I think?
>
> Most likely, the reason that that desktop/source/deployment code only
> checks against existing extension libraries but not built-in ones is
> that that was never a use-case the code was designed for.  I do not
> know, though,whether there are any gotchas on the BASIC side that
> would be enabled by your proposed change.
 I'm not sure, I know any duplicate symbols (and it can happen in
libreoffice basic) can cause unexpected and suprising results, imho they
shouldn't be allowed but that's another story. I'm still not sure what
actually happens when such a scenario as above is forced, is the 'old'
library removed or not, if not does the 'old' library actually get
compiled even, what happens later when upgrading, does it set a
precedent for binary extensions to be able to replace 'system'
components (if that isn't already possible) etc.

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

Re: Access2Base - New release

[Noel Power]

On 19/05/14 14:59, Lionel Elie Mamane wrote:
> On Mon, May 19, 2014 at 11:06:46AM +0100, Noel Power wrote:
[...]
> Access2Base is considered a part of the core isn't it? it isn't
> shipped as an extention, it is shipped as part of the product, (...)
> Access2Base is either part of the product or it's not.
> I don't think this was a very conscious decision. Access2Base started
> its life as an extension that got integrated into LibreOffice, but is
> still available as an extension for other branches / forks of the
> code. It got shipped as part of the product since that was easier to
> set up and LibreOffice was (my perception) moving away from bundled
> extensions anyway.
IMHO moving away == moving functionality into the core => stable api
with the same rules as the rest of the code
>
>> it seems ato me that you are trying to get around the rules of no-new
>> features etc. by exploiting the extension mechanism.
> No, extensions are *very* *much* *designed* to allow addition of new
> features to LibreOffice!
sure extensions are very much designed to add new functionality (also
independantly updateable from thing (libreoffice) they extend ) they are
not designed to replace in an uncontrolled way core functionality, that
leads to a maintenance (&security??) nightmare scenarios
[...]
>> But in anycase although Access2Base is part of the core, part of
>> the product etc. it is afaik completely selfcontained (and
>> essentially a separately maintained subsystem) in this case I think
>> there is a good argument to bend the rules regarding updating the
>> version of Access2Base shipped, we already do that occasionly I
>> think?
> Well, that means we ship a changing API into our stable line (I mean
> patchlevel updates). I'm not comfortable with this. I'd be far much
> comfortable if people that wanted the changed API installed it
> explicitly as an extension.
[...]
then Access2Base should be an extension, they are designed with that in
mind, a bundled extension would have been a better choice, it at least
gives the illusion of being part of the product whilst giving more
flexibility. I don't know what the answer is here, personally I don't
have a problem with Access2Base being updated given what I said above,
but I don't believe replacing non-extension code (be-it binary or
script) with extension code is a good idea
>
>> does it set a precedent for binary extensions to be able to replace
>> 'system' components (if that isn't already possible) etc.
> Maybe I'm naive, but I'm in principle OK with that; an extension that
> breaks something when doing that gets to pick up the pieces.
>
pity then the poor developers trying to debug some crazy (and unobvious)
mixture of unsupported and supported (core) code not really realising
what is what

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

Re: Access2Base - New release

[Noel Power]

On 28/05/14 12:11, Lionel Elie Mamane wrote:
> On Mon, May 19, 2014 at 04:33:03PM +0200, Michael Stahl wrote:
>> On 19/05/14 15:59, Lionel Elie Mamane wrote:
[...]
>> "addition", but not "replacement", especially not "potentially
>> partial replacement".  with a "bundled extension" it would work to
>> replace it, because there LO knows the "boundaries" of what is being
>> replaced and can disable the whole bundled extension;
> I don't buy that it does that.
huh? extensions are built on top of and using the stable (well as stable
as libreoffice ever is) api, if you allow 'other' extensions to change
the 'core' api (with no way of knowing that the base system has been
changed) then well good luck with stability
>  If (bundled, or system-installed)
> extension "Foo" ships Basic libraries "Quux" and "Frobotz", and then
> the user installs extension "Bar" that ships Basic library "Quux", but
> not "Frobotz", then IMO the result is:
>
> 1) Basic library "Quux" from "Foo" is disabled.
> 2) Basic library "Frobotz" from "Foo" stays enabled.
> 3) Basic library "Quux" from "Bar" is enabled.
you are just stating how you want it to work, it's not a justification
>
>> but when replacing something that's built-in you can't assume that
>> it was ever designed to be replaced,
> It's FLOSS, it can always be replaced by a binary-compatible fork of
> it: a bug fixed, a feature added in a compatible way, a version that
> logs what the user does, a version that bills printed pages to the
> appropriate customer (by hooking into the printing function), ...
I don't even understand the argument, but... hey if you are saying that
someone can build various libraries and components and shove them into a
libreoffice installation then sure, they could do it, they are free to
do it but would you actually provide support for them to do that
>
>> you can end up with a mixture of built-in files and extension files
>> loaded that doesn't work.
> Yes.
>
>> if you want to bundle something that can be replaced by the user, do
>> it as a bundled extension.
> How about we put an exception in the code that only Basic (and
> Dialog?) libraries, or maybe only access2base, can be overridden by an
> extension? I don't think we ship any actual feature as Basic code -
> only examples (I'm less sure about Dialog libraries), so "it will not
> hurt".
why should there be an exception for Basic, there is no possibly
justification for that, oh, hang on I think I can hear it coming, "well
we don't really ship anything in terms of api with Basic at the moment,
so... it won't break anything" But it does, there are at least library
functions and wizards, but... in anycase the principle is the same. It
all boils down to the fact that if you want to provide updated and
incompatible api then you don't do that in the release branches, that's
what master is for, if you want something (e.g. access2base) to act like
and extension (that provides a different update strategy) then just make
it an extension
>
> And here, the boundaries are very clear, "known" by the code and
> handled well by the code: one Basic library.
>
> I mean add to the code a case like this:
>
>|| sOriginalUrl.match("$(INST)/share/basic")
>
> or
>
>|| sOriginalUrl.match("$(INST)/share/basic/Access2Base")
>
>
> Or should it be something like
>
>|| 
> sOriginalUrl.match("vnd.sun.star.expand:$BRAND_BASE_DIR/share/basic/Access2Base")
> or
>|| 
> sOriginalUrl.match("vnd.sun.star.expand:$BRAND_BASE_DIR/$BRAND_SHARE_SUBDIR/basic/Access2Base")
>
> ?
>
> It avoids us demoting Access2Base to a bundled extension.
yeah, my feeling here is avoiding this perceived 'demotion' of
Access2Base is what this debate is about and not a genuine technical
need or bug. I hope I am wrong. Truth is though bundled or not,
users won't care or notice. Surely the fact that it is supported, it is
isn't it? (otherwise what's the point??) is IMHO the important point or
issue for users. That would be what discriminates it from other bundled
extensions (I bet many users expect are all bundled extensions are
supported regardless)
>
>
> On Mon, May 19, 2014 at 04:39:36PM +0100, Noel Power wrote:
>> On 19/05/14 14:59, Lionel Elie Mamane wrote:
>>> On Mon, May 19, 2014 at 11:06:46AM +0100, Noel Power wrote:
>> [...]
>>> Access2Base is considered a part of the core isn't it? it isn't
>>> shipped as an extenti

Re: Access2Base - New release

[Noel Power]

On 12/06/14 16:16, Lionel Elie Mamane wrote:
> On Fri, Jun 06, 2014 at 09:55:05AM +0100, Noel Power wrote:
>> On 28/05/14 12:11, Lionel Elie Mamane wrote:
>>
[...]
>> you are just stating how you want it to work, it's not a
>> justification
> No, that was my prediction from reading the code that implements
> that. I now *tested*, in plain 4.2.4.2 (Debian amd64 package), that it
> indeed behaves in that way, by:
>
> 1) Installing "Foo" as a user extension
> 2) Observe result: Libraries Quux And Frobotz (from Foo) are enabled
>and available.
> 3) Install "Bar" as a user extension
> 4) Observe result:
>* Library Frobotz is still there and available (from Foo)
>* Library Quux from Foo is nowhere to be found in the Library
>  browser.
>* Library Quux from Bar is enabled and present, and can be used.
I took it (or thought that I read it) that the description above was how
you wish it to behave when installing a user extension against an
existing share Basic Library.
[...]
>> Security, I mentioned it in a previous mail, what you are suggesting is
>> basically allowing the user to rootkit the libreoffice api.
> I'm surprised you consider LibreOffice a security barrier /
> enforcer. I don't. As I see it, factually, it is an application that
> runs with the user's identity and privileges, that is under the
> control of the user and of anybody that subverts the user's
> environment. If you are thinking in terms of security, well LD_PRELOAD
> / LD_LIBRARY_PATH allows overriding any binary part of LibreOffice
> anyway. As does LD_PRELOAD'ing a library that redefines open() to
> override any part of LibreOffice (Python code, Basic, dialog .ui,
> ...).
sure you can do anything you want, but I am not talking about a
convoluted and technically complicated scenario like wrapping various
libreoffice libraries. It's clear you are comfortable with the idea that
a user extension can override the core, a hypothetical example being say
of a user installing a user extension that overrides the allows a user
XProtectable so they can easily unlock any spreadsheet to unhide their
bosses salary. But... the issue here isn't the silly example it's the
fact that libreoffice itself uses the api, it expects the code it calls
via the api to be the code it shipped with and to behave exactly how it
expects, this is one of the boundaries (or at least how I read it)  that
Michael mentioned previously and one of the invariants the core expects.
The key is in the name, extensions extend, not override
>
>> In the binary case the possibilities should be clear. But even if
>> Libreoffice didn't ship any basic libraries as part of the core it
>> wouldn't change the fact that enterprises normally deploy all of
>> their macros in share, I doubt they would wish that a user could
>> 'override' any libraries (including Access2Base that possibly some
>> of their macros have a dependency on)
> Enterprises are free, if they so wish, to forbid their employees to
> install an extension that overrides a part of core, or otherwise
> override a part of LibreOffice core.
first they need to know that they need to tell their users not to do
that, then they got to believe that users will obey them or understand,
but I suppose they could always customise libreoffice so that the
extension functionality is not available or go through a hundred other
hoops they never had to know about or consider before...
>
> I think that a (semi-)structured / (semi-)controlled (that is,
> extensions) way of overriding more-or-less arbitrary parts of a
> program is much better than having to resort to LD_PRELOAD /
> LD_LIBRARY_PATH, but that's not my fight, and (obviously) not a change
> we'd introduce in a stable branch.
>
see above, and I fail to see how lowering the barrier to for people to
do bad (intentionally or not) things is a good idea,

Anyway, since binary overriding isn't on the table yet my main concern
was with basic where you have can have Libraries deployed in share by
the enterprise, and with the previous proposal the possibility that a
user could with an extension override say the corporate authorisation
macros or whatnot.
Above you concede that allowing this in Basic generally is probably not
a good idea. But... still you propose to special case this overriding
for Access2Base, I can't see how it is any different, the same argument
holds and users can easily break deployed macros depending on the
'installed' version of Access2Base.
> FWIIW, an enterprise could also wish to deploy a newer Access2Base
> version without upgrading to LibreOffice Fresh (because they are
> enterprise, they want Stable), because it suits their newly developed
> macros bette

Re: Access2Base - New release

[Noel Power]

On 13/06/14 11:45, Lionel Elie Mamane wrote:
> On Fri, Jun 13, 2014 at 09:44:42AM +0100, Noel Power wrote:
>
>> Anyway, since binary overriding isn't on the table yet my main
>> concern was with basic where you have can have Libraries deployed in
>> share by the enterprise, (...)
> I'm not sure what "deployed in share" means. An extension installed
> "for all users" / with "unopkg --shared"? I sense that this is not
> what you mean. Maybe by just dropping a directory + files in
> LibreOffice's $(instdir)/share/basic/ ? I hadn't thought of that as a
> good way to extend LibreOffice... Is that really how it is done?
how is Access2Base delivered? (I actually don't know) but I presume it
like the wizards and other basic utilities that it is located in
$(instdir)/share/basic.
That is the also location where Enterprise's will also deploy their
macros so that they are picked up by *all* users (or at least that is
where they used to do that)
>> and with the previous proposal the possibility that a user could
>> with an extension override say the corporate authorisation macros or
>> whatnot.
> I was exploring / discussing the various possibilities, not having yet
> myself formed an opinion on which one is the best. Given the
> resistance to something general, I made up my mind on something
> "narrow and specific", at least "right now".
>
> (Anyway the user can still "override say the corporate authorisation
>  macros or whatnot" by making a copy of LibreOffice into another
>  folder, changing that copy and running that copy...)
just because there may be some way around this particular example (and
it is only one example) doesn't mean that there is no value in the
application trying to protect it's own internal integrity and certainly
it is no reason to give up it's internal integrity by inviting users to
override it's core (or installed) functionality.
In some corporate environment users don't have access to shell, it's
also very likely that most users don't have the skills to deeply modify
their environment (LD_PRELOAD etc.) I don't believe that just because an
application can in some circumstances (with willful effort by a user) be
led to run code it doesn't expect that it logically follows one should
change the application to actively support running unexpected code
(which is what you are suggesting)
>> Above you concede that allowing this in Basic generally is probably
>> not a good idea. But... still you propose to special case this
>> overriding for Access2Base, I can't see how it is any different, the
>> same argument holds and users can easily break deployed macros
>> depending on the 'installed' version of Access2Base.
> Yes, especially when installing an older version of Access2Base than
> the one in core. Why is that a problem? They broke it, they can pick
> up the pieces. Similarly, the user can also get themselves in great
> trouble by writing a bomb threat in Writer and mailing to the White
> House. We don't have any protection against that.
The problem is that to get into that situation in the first place you
have had to allow the user to override the core behaviour with an
extension, isn't that the what this discussion is all about (wasn't the
first question in this thread about why there is a piece of code that
enforces that such a scenario doesn't occur)? It's not about Access2Base
at all (although it is obviously the trigger) it's about changing the
defined behaviour (even in a limited way that effects just Access2Base)
where user extensions can override/replace core functionality
It's quite simple (all smokescreens about user freedom etc, aside), once
this change is in a release it becomes accepted behaviour. It's then
only a matter of time before the expectation is that it should work the
same way for the rest of Basic, and then for consistency an expectation
of the same behaviour for binary extensions an so on. Given your
expressed views that this is how you would like the application to
behave anyway, I find this change worrying to say the least ( caveat
being if indeed the behaviour is accepted as desired )
> The user can also make their environment unusable by changing the UI
> language to a language they don't understand. And a myriad other
> things...
so what?
> On a different tack, one difference is that "getting the latest
> Access2Base" is something available to users of other branches of the
> code (LibreOffice 4.1 and earlier, Apache OpenOffice, ...), so here we
> are removing a "competitive disadvantage".
this isn't a justification of why the present behaviour should be broken
just an explanation why it is convenient for

Re: Of customisability, overridability and FLOSS/LibreOffice's mission

[Noel Power]

On 13/06/14 11:08, Lionel Elie Mamane wrote:
> On Fri, Jun 13, 2014 at 09:44:42AM +0100, Noel Power wrote:
>> On 12/06/14 16:16, Lionel Elie Mamane wrote:
>>> On Fri, Jun 06, 2014 at 09:55:05AM +0100, Noel Power wrote:
>>>> On 28/05/14 12:11, Lionel Elie Mamane wrote:
>>>> Security, I mentioned it in a previous mail, what you are suggesting is
>>>> basically allowing the user to rootkit the libreoffice api.
>>> I'm surprised you consider LibreOffice a security barrier /
>>> enforcer. I don't. As I see it, factually, it is an application that
>>> runs with the user's identity and privileges, that is under the
>>> control of the user and of anybody that subverts the user's
>>> environment. (...)
>> (...) It's clear you are comfortable with the idea that a user
>> extension can override the core, a hypothetical example being say of
>> a user installing a user extension that overrides the allows a user
>> XProtectable so they can easily unlock any spreadsheet to unhide
>> their bosses salary.
> (This is an interesting point, but is besides the access2base
>  discussion. I'm forking it into a separate subthread.)
>
> That's actually a good example, and similar to what I do with my PDF
> readers to allow me to copy (for paste for quotation) / print /
> ... (that is, ignore the permissions bits).
actually it is not a good example, it is, as I said a 'silly example'
> In your example, the boss's salary is available with the combination
> of "unzip / xmlindent / less" anyway,
[...]
oh please, give me a break, I simply wanted to illustrate how trivially
the application's internal consistency with regard to the api has been
broken by allowing 'users' to 'replace' api with an extension. How you
might otherwise achieve the same result (bypassing of the password in
this particular example) is completely and utterly irrelevant
[...]
>  
> Now that I think of it, LibreOffice has an indirection level that
> "service names" are mapped to "implementation names" by way of
> *configuration* files, the ".component" files. So I wouldn't be too
> surprised that by mapping a service name that is usually provided by
> core to an implementation in an extension, the kind of things you
> describe is *already* *possible*. Any LibreOffice code (even from
> core) instantiating the service would get the implementation from the
> extension (independent on the particular example, whether XProtectable
> is implemented in a service or not...). Never tested it, though.
I would be exceedingly surprised that this is the case, it would mean 1
of 2 things
a) a bug
b) or that by design a user is able to replace binary core functionality

if a) then that is a hole that would need to be *immediately* fixed imho
if b) then I would respectfully disagree with that behaviour but I would
also recognise for reasons of consistency that my whole problem with
your patch allowing "even in a limited way" for a user to override core
behaviour with an extension is baseless. However... I sincerely doubt
that is the case, I would love to hear some definitive expert opinion on
that say from Stephan, is that true then?, a user should be able to
replace internal component services (e.g. document loading, password
api, anything...) with a user extension.
>> But... the issue here isn't the silly example it's the fact that
>> libreoffice itself uses the api, it expects the code it calls via
>> the api to be the code it shipped with and to behave exactly how it
>> expects, this is one of the boundaries (or at least how I read it)
>> that Michael mentioned previously and one of the invariants the core
>> expects.
> Taking the role of the user, *this* *is* *my* *computer* and my
> installation of LibreOffice. Let me screw it up if I want. I see us
> (LibreOffice project, Free Software movement in general) as being
> about empowering the *user*.
 if this is your computer then you can already do what you want,
you can already screw it up, you have root access. It has nothing to do
with the 'empowering the user' it more about the right the application
has to try and apply it's own rules for maintaining it's own internal
consistency, if you don't like that, you can fork the project, you can
download the source, you can change the behaviour, you already can do
what you want (including like you say weird and wonderful things with
LD_PRELOAD, making a copy etc. etc.)
> You want to throw away my bug reports because I have an extension that
> overrides the core? Fair enough (that's similar to Linux (the kernel)
> throwing away bug reports from tainted kernels, for examp

Re: getting help with Bug 71248 - Excel VBA: ActiveDocument is not tracking currently selected document

[Noel Power]

On 09/09/14 15:55, Justin Luth wrote:
> Are sc/source/ui/vba/vbaapplications.cxx and
> vbahelper/source/vbahelper/vbahelper.cxx ONLY used for MS VBA
> compatibility, or does it also affect StarBasic etc?
it only affects MS VBA compatability
>
> I have identified that the vbahelper function getCurrentDoc() is
> broken, causing bug 71248, and suggested a band-aid solution for it in
> the comments.  However, I don't know the larger implications of
> replacing getCurrentDoc with getThisExcelDoc.  In practice, THIS and
> CURRENT are likely the same thing
no, 'THIS' refers to the document running the macro and 'CURRENT' is the
'active' document. E.g. if a macro in foo.xls opens existing ba.xls,
'THIS' refers to foo.xls and CURRENT refers to ba.xls (after the
document is raised and opened and is still on top)
> , but perhaps the current implementation is necessary and working for
> someone...
>
> I'm still going to try to figure out how to properly fix
> "getCurrentDoc" but so far I don't really understand what it is doing.
I haven't looked at that code in a long time so feel free to dive in
there, iirc basically 'THIS' is tracked in the uno context (VBA has a
custom uno context), CURRENT should be accessible as a UNO global
variable inserted in to 'application' StarBasic. It sounds like somehow
somewhere getCurrentDoc either is no longer searching for the correct
variable, or its no longer getting updated (used to be updated somewhere
in sfx2/.../objxstor.cxx (try searching for 'ThisComponent', it's bound
to be handled somewhere close to that)
There used to be some confusing (but necessary) indirection to handling
this e.g. because Libreoffice is a single application (and VBA support
was trying to support both Word & Excel) the code in objxstor.cxx needs
to find out which variable in StartBasic to update, in the case of a
Word document it would need to update StarBasic variable named
'ThisWordDoc' & for Excel 'ThisExcelDoc' variable (both can exist
simulateously if multiple documents xls/doc are open).  That is my
recollection how this used to work, it is possible imports from AOO code
have trampled/changed this possibly in an incompatible way
> If someone who actually knows how to program and submit patches could
> take over from this point, that would be great.
>
> suggested Band-aid fix:
> uno::Reference< frame::XModel >  getCurrentExcelDoc( const
> uno::Reference< uno::XComponentContext >& xContext ) throw
> (uno::RuntimeException)
> {
> return getThisExcelDoc( xContext );
> }
I think this would be a bad idea, getCurrentExcelDoc needs fixing to
return the topmost open Excel document
> Thanks,
> Justin
> https://bugs.freedesktop.org/show_bug.cgi?id=71248
>
thanks,

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

Re: [Libreoffice] Examing unicode strings in gdb (while debugging LibO)

[Noel Power]

On Thu, 2010-11-18 at 09:30 +, Caolán McNamara wrote:
> On Thu, 2010-11-18 at 00:17 +0100, Miklos Vajna wrote:
> > On Wed, Nov 17, 2010 at 04:44:19PM +, Caolán McNamara 
> >  wrote:
> > > print dbg_dump(string)
[...]
> Without checking myself, are those pu macros horrifically slow or
> speedy ?

horrifically slow


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

Re: [Libreoffice] use dbglevel=1 for --enable-debug builds?

[Noel Power]

On Thu, 2010-11-18 at 13:01 +0100, David Tardon wrote:
> Hi all,
> 
> currently, configuring with --enable-debug means that dbglevel is set to
> 2, which in turns means that OSL_DEBUG_LEVEL is set to 2. This enables
> all the OSL_ debugging stuff there is (well, maybe there is some really
> rare stuff that is only enabled for OSL_DEBUG_LEVEL > 2, but I don't
> know about any), which spews unbelievable amount of stuff on stdout.
> 
> If we used dbglevel=1 as default, only assertions would be enabled and
> the output would be much more manageable (practically nonexistent in
> most cases, from my experience). 
I never use --enable-debug ( some my opinion aint worth much ) but the
above seems more sensible that adding even more options

Noel


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

Re: [Libreoffice] Building problem, DEBUG_ELEMENT when DEBUG is off

[Noel Power]

On Tue, 2010-11-23 at 23:51 +0100, Joost Eekhoorn wrote:
> Hi,
> I had a problem in writerfilter, building without DEBUG.
> 
> I did some local hack with #undef, to get it building again.
[...]
I fixed a bug in this area, should be good now

thanks,

Noel

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

Re: [Libreoffice] [PATCH] Basic Currency Issues #i31001# to #i107277#

[Noel Power]

On Tue, 2010-11-30 at 01:35 -0800, John LeMoyne Castle wrote:
> OOo Basic has had issues with its fixed point Currency type for a long time.  
> I think the Basic Currency type deficiencies create a bar to adoption in the
> minds of business users.  
> Nothing says 'run away' like mangling the money numbers. 
Agreed !! and thanks for looking at this
> 
> Currency is presently implemented with a 64bit struct of 2 Int32s and the
> math is done by the tools/bigint class. 
> The attached patches replace the code to implement the Currency type with an
> ISO C99 compiler supported 64 bit integer. 
> In fact, the 64 bit int was already a member of the Sbx core data union.  
> Main patch makes 1/2 kLOC reduction in sbx.
yeah, so you will have to give me some time to look at this ( not really
familar with the Currently type ) and I need to somehow filter out the
whitespace changes too ( probably I can regenerated the patch to do
that. 

regards,
Noel

p.s. are you aware of more iz issues related to this other than the 2
mentioned in the subject? be good to have as much test data as possible


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

Re: [Libreoffice] [PATCH] Basic Currency Issues #i31001# to #i107277#

[Noel Power]

sending again ( whilst attempting to preserve the threading this time )
On Tue, 2010-11-30 at 01:35 -0800, John LeMoyne Castle wrote:
> OOo Basic has had issues with its fixed point Currency type for a long
time.  
> I think the Basic Currency type deficiencies create a bar to adoption
in the
> minds of business users.  
> Nothing says 'run away' like mangling the money numbers. 
Agreed !! and thanks for looking at this
> 
> Currency is presently implemented with a 64bit struct of 2 Int32s and
the
> math is done by the tools/bigint class. 
> The attached patches replace the code to implement the Currency type
with an
> ISO C99 compiler supported 64 bit integer. 
> In fact, the 64 bit int was already a member of the Sbx core data
union.  
> Main patch makes 1/2 kLOC reduction in sbx.
yeah, so you will have to give me some time to look at this ( not really
familar with the Currently type ) and I need to somehow filter out the
whitespace changes too ( probably I can regenerate the patch to do
that. )

regards,
Noel

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

Re: [Libreoffice] [PATCH] Basic Currency Issues #i31001# to #i107277#

[Noel Power]

Hi John,
On Tue, 2010-11-30 at 04:09 -0800, John LeMoyne Castle wrote: 
> Hi Noel, 
> Sorry about the whitespace problem.  Is it because I cleaned up the header
> formatting? I would regenerate the patch but I don't understand clearly what
> the whitespace problem is.  Please let me know what I did wrong.
Don't know if you did anything wrong as such just that the reformatting
and reorderings of various things ( like format of some macros )
position of data definitions in the SbxValues data struct etc. make lots
of noise for trivial changes. just takes more a little more time to
unravel :-)
> 
> There are at least two more iz issues - they are in the primary commit
> header: 54049, 91121, 
> There are also others closed as dups that may have more test cases.  Look at
> any op with operands near or results just above (2^31)/1 or 2^31.  There
> are a few 100k of examples in the test sheets for Beta3 in CurTestODS.ods. 
good to know, thanks again

Noel


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

[Libreoffice] [PUSHED] Re: more SAL_N_ELEMENTS

[Noel Power]

Hi Kenneth,

thanks for the patch, perfect as far as I could see so it is pushed

thanks,

Noel

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

Re: [Libreoffice] Patch for i115495: import problem with centered rtl tables from word

[Noel Power]

On Sun, 2010-12-05 at 23:28 +0200, Lior Kaplan wrote:
> Hi,
> 
> Can any one look at the small patch attached to
> http://www.openoffice.org/issues/show_bug.cgi?id=115495
> 
> It would be nice to have this fixed in LibO 3.3, as this annoys a lot
> of users of the the RTL languages.
> 
> Thanks
> 
> Kaplan

Looks like this is already in master

Noel


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

Re: [Libreoffice] [PATCH] Basic Currency Issues #i31001# to #i107277#

[Noel Power]

Hi John,
On Tue, 2010-11-30 at 04:22 -0700, Noel Power wrote:
> > Main patch makes 1/2 kLOC reduction in sbx.
> yeah, so you will have to give me some time to look at this ( not really
> familar with the Currently type ) and I need to somehow filter out the
> whitespace changes too ( probably I can regenerated the patch to do
> that. 

Nearly done with this, just need to write down some stuff in a mail etc.
I do think we need this patch but imo it deserves a feature branch as 
  a) there are some things I would like initially conditionally compile
out ) 
  b) I also made some changes [1] so I think it makes sense for me to
create the feature branch ( I will do that today )
  c) there are some issues to do with stream handling that need
addressing and at least further testing ( in fact I am not even sure
that some of this stuff is needed anymore... but that is another story )

So, again thanks for this patch I do think it is useful and improves
some very sadly broken functionality. Anyway I will have more detailed
comments for you later, be a little more patient with me :-)

thanks,

Noel
[1] - one change I made was in ImpCurrencyToString where I wanted to use
existing code to build the string ( hehe I just noticed that I
completely broke handling of negative numbers )
Also, it's entirely possible that I even added more bugs ( my head
started to hurt with this patch.. very niggledy ) but your test document
results at least are the same after my changes ( your good test document
did catch some horrific error that I made too :-) )

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

Re: [Libreoffice] [PUSHED] [PATCH] Clean code at impress

[Noel Power]

Hi Kayo
On Sun, 2010-12-05 at 14:37 -0800, Kayo Hamid wrote:
> Sending for review, cover sd/slideshow/*
> 
> revol_
> 
Pushed, I didn't commit the changes to
slideshow/source/engine/sp_debug.cxx as I find those comment telling
what #ifdef the #endif closes very useful. Thanks muchly for the nice
patch!!

Noel

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

Re: [Libreoffice] [PATCH] Basic Currency Issues #i31001# to #i107277#

[Noel Power]

 Hi John,

On 30/11/10 09:35, John LeMoyne Castle wrote:


Currency is presently implemented with a 64bit struct of 2 Int32s and the
math is done by the tools/bigint class.
The attached patches replace the code to implement the Currency type with an
ISO C99 compiler supported 64 bit integer.

Absolutely, using 64bit integer types makes sense

In fact, the 64 bit int was already a member of the Sbx core data union.

[...]

http://nabble.documentfoundation.org/file/n1991648/0001-Basic-Currency-64bit-impl-fixes-i31001-to--libs-core.patch.tar.gz
0001-Basic-Currency-64bit-impl-fixes-i31001-to--libs-core.patch.tar.gz
This is integrated now in the feature branch  feature/currency-64bit, in 
the main I am really happy with this patch, here is a summary of things 
I changed/modified etc.


First off the patch didn't build for me, at a wild guess I'd say maybe 
you built this on a 32 bit system ( as the error seemed 64 bit related.. 
but I am not completely sure :-) ) error was..


 ../../inc/basic/sbxvar.hxx:92:5: error: 
'SbxValues::SbxValues(sal_Int64)' cannot be overloaded
./../inc/basic/sbxvar.hxx:87:5: error: with 'SbxValues::SbxValues(long 
int)'

dmake:  Error code 1, while making '../../unxlngx6.pro/obj/sbintern.obj'

in anycase I removed the associated SbxValues constructor as it seemed 
to enforce SbxValues::SbxValues(sal_Int64) would set the type of 
SbxValues as SbxCURRENCY. I don't think it's a good idea to enforce 
that, sal_Int64 just happens to the the data type we use for the 
SbxCURRENCY type, it isn't indicative of the type itself.


I reverted the change below

-#define SbxMININT(-32768)
+#define SbxMININT   (-32767)

I thought perhaps this wasn't intentional until I saw similar in the 
test document, did you have a specific reason for changing that, imo we 
can't change that as it is a hard limit. ( btw I changed it in both the 
test document and the code and see no difference in the generated results )


I removed various comments that looked I guess like internal notes to 
yourself, please have a look and ensure that I didn't remove something 
you really want to keep. As an example ( from basic/inc/basic/sbxdef.hxx )


-#defineSBX_MAXINDEX0x3FF0
-#defineSBX_MAXINDEX32SbxMAXLNG
+// ?can we remove this ~16k limit on array index/size?
+#define SBX_MAXINDEX0x3FF0
+#define SBX_MAXINDEX32  SbxMAXLNG
+

as an aside of course Arrays and such are no longer limited ( afaik ) by 
16bit  I guess we probably could get rid of the many side by side 16 and 
32 bit variants of some method ( see Get32 & Get methods in SbxArray ) 
But that would be another task ( and I am not really sure the effort to 
do that is worth it with so many other problems to look at )


in  sbxToUnoValueImpl ( convert to uno without known target class ) in 
basic/source/classes/sbunoobj.cxx previously an Double or Float basic 
value ( within the range of an int ) was transfered as a the smallest 
type e.g. sal_Int8, sal_Int16 or sal_Int32 values greater were 
transferred as Float or Double
   * there is a bit possibility that we could break existing code, care 
needed here I thing ( transferring the value as sal_Int64 might seem 
natural but hyper seems to be rarely used in uno interfaces ) not sure 
about this, I am tempted to just let things break ( 'cause they seem 
wrong in the first place ) Otoh the comments in the code do suggest 
converting to int is intentional. ( erring on the side of caution it's 
#ifdefed out ( with #if MAYBEFUTURE ) at the moment )


I see you have wiped the following types SbxLONG64 & SbxULONG64 from the 
face of the earth ;-) I can't see that these are used anywhere and I 
would guess they are handled probably in the binary filter code only. So 
I hope this change is ok too.


in basic/source/runtime/methods1.cxx you have added incomplete 
read/write support for types SbxSALINT64 & SbxSALUINT64,  lets not add 
support to persist a new type incompletely ( to easy to forget about ) 
so I've  #ifdefed it out ( with #if IMPLEMENTATION_READY )


I fixed a couple of identical treatment of SALxINT64 & SbxCURRENCY types 
e.g. sbxbyte ( there were others too ) these break things like



dim a as currency
dim b as byte:w

a = 1
b = a '<- overflows here ( clearly incorrect )

other types are similarly affected

similar to the above there I also fixed some mismatch with the Get/Put ( 
maybe some of this was pre-existing, I wouldn't be suprised )


I re-added some checks for removed limit checks ( actually I hope that I 
didn't make things worse here as all this similar methods started making 
my head spin, it's quite possible I screwed something up )


changed signature of ImpPutCurrency( sal_Int64& n ) 'tis a bad idea 
'cause the incoming could be unexpectedly changed ( I realise this is an 
oversight due to the fact that previously this method had an object 
passed in the signature ) perhaps I am being paranoid... but anyway I 
changed it


fixed some missing breaks in var

Re: [Libreoffice] [PUSHED][PATCH] RTL_CONSTASCII_USTRINGPARAM for dbaccess/source/ui/dlg

[Noel Power]

Hi Julien
On 06/12/10 21:40, Julien Nabet wrote:

Hello,

Here's a small patch for dbaccess/source/ui/dlg

This is a strange one, it seems SERVICE_UI_FOLDERPICKER is not a simple 
const char but an instance of ConstAsciiString ( defined in 
dbaccess/source/inc/constasciistring.hxx ) so I changed the patch as follows


-::rtl::OUString sFolderPickerService = 
::rtl::OUString::createFromAscii(SERVICE_UI_FOLDERPICKER);
+::rtl::OUString 
sFolderPickerService(SERVICE_UI_FOLDERPICKER);


it seems that there is already an operator to return an OUString& . 
Caolan, this is a new one on me, you seen it before? Does that look like 
we might have problems if we converted similar definitions as  
presumeably RTL_CONSTASCII_USTRINGPARAM would try to take sizeof( 
ConstAsciiString )  in that case


I committed the variation above


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

Re: [Libreoffice] [LibreOffice] [Patch] Fix for i#108228: Bool has negative sign when used in Writer formula expression

[Noel Power]

Hi Mattias/Michael
On 08/12/10 11:20, Michael Meeks wrote:

Having said that, this really doesn't seem ideal. I wonder what the
compatibility impact of changing it would be [ a chestnut for Noel to
consider I suspect ].
From the basic point of view as Mattias says ( and afaics ) this 
doesn't change the core basic behaviour at all, the change affects 
writer formula processing. Aside from the seemingly weird 
SbxValue/SbxValues Put/ImplPUT inconsistencies I think basic's boolean 
handling is afaik working ( e.g. Boolean is always -1 ) and yes this is 
aping VBA. If that isn't the case I'd be really interested in seeing an 
example where it doesn't work ( if those sort of bugs exist they need 
reporting and fixing )
For this patch I am not familiar with the writer bits involved, so I am 
not sure if there would be some side-affects. Personally I'd like to 
understand why the "explict check in the output" doesn't get applied in 
this case, I'll at least try and see where this is done to see if I can 
see any obvious risks. If I don't see anything ( or nobody else objects 
) I guess we can shove this into master, I'll look a little more at this 
later. Thanks again for looking though, nice work !!



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

Re: [Libreoffice] [LibreOffice] [PUSHED] [Patch] Fix for i#108228: Bool has negative sign when used in Writer formula expression

[Noel Power]

On 08/12/10 16:22, Noel Power wrote:
For this patch I am not familiar with the writer bits involved, so I 
am not sure if there would be some side-affects. Personally I'd like 
to understand why the "explict check in the output" doesn't get 
applied in this case, I'll at least try and see where this is done to 
see if I can see any obvious risks. If I don't see anything ( or 
nobody else objects ) I guess we can shove this into master, I'll look 
a little more at this later. Thanks again for looking though, nice work !!
Looking at this today I was worried that changing the type of the bool 
variable might cause problems ( because it's possible the variable/value 
would be reused in an expression ) but looking further in the writer 
code every where a boolean operation is performed it seems the variables 
userd are converted to the boolean type appropriately, so to me ( and 
really I have only a limited clue ;-) ) I think this is ok, so pushed


thanks again

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

Re: [Libreoffice] [PATCH] Basic Currency Issues #i31001# to #i107277#

[Noel Power]

Hi John,
On 06/12/10 17:58, Noel Power wrote:

 Hi John,

[...]
in  sbxToUnoValueImpl ( convert to uno without known target class ) in 
basic/source/classes/sbunoobj.cxx previously an Double or Float basic 
value ( within the range of an int ) was transfered as a the smallest 
type e.g. sal_Int8, sal_Int16 or sal_Int32 values greater were 
transferred as Float or Double
I removed the '#if MAYBEFUTURE'  I put around the code above. After some 
poking around and also looking at the dev guide it seems this code is to 
handle the case where the expected type ( on the uno side ) is unknown, 
in this case the value is down cast to the smallest ( non-internal ) 
basic type which is then converted to an uno type ( or at least that's 
my current take on the intention )


in basic/source/runtime/methods1.cxx you have added incomplete 
read/write support for types SbxSALINT64 & SbxSALUINT64,  lets not add 
support to persist a new type incompletely ( to easy to forget about ) 
so I've  #ifdefed it out ( with #if IMPLEMENTATION_READY )
I'm not sure it is needed but I enabled this ( and created a new stream 
operator to handle the 64 bit type )


I rewrote the ImpCurrencyToString, I took note of your suggestions re. 
optimizations whilst trying to point you to some existing 
functionality available from the String classes as its better to avoid 
generating even more number to string code. Somewhere between trying 
to write down some explanations and suggestions I'm afraid I found 
myself actually rewriting this and then I found mistakes in my 'new' 
implementation... couldn't help myself. Anyway hopefully some of the 
code there shows some other possibilities to achieve the same thing 
reusing some of the existing LO classes. ( I have no doubt that there 
probably is some bugs in my implementation too ). Additionally I am 
still very uncertain about   ImpStringToCurrency() changes to use 
Locale specific separators. Also not sure about the local seperator 
logic, the heuristics used I think might give an undesired result 
under some circumstances. I don't want to rule it out but for the 
moment I would feel happier with the locale specific part being 
#ifdefed out ( so I did that with some more   #if MAYBEFUTURE blocks ).
I also rewrote the ImpStringToCurrency a little, actually more like I 
combined yours and the old implementation a little, mostly I just 
removed the home grown parser in favour of using the existing 
string->number conversions in OUString. Like the ImpCurrencyToString I 
have enclosed the locale specific functionality in a '#if MAYBEFUTURE' 
block. I removed the acceptance of blank characters in the middle of a 
string as VBA regards this as an error and the old implementation just 
truncated the number ( e.g. "12345 6" would end up as 12345 ), I raise 
an error in this case now. Hopefully if this breaks existing macros it 
will only break code already deeply in error ( from mis-interpreting the 
string/number  )  The other reason for removing the parser is if/when we 
do apply some locale specific separator processing I would love to 
reuse/share whatever calc currently uses.



SbxValue::LoadData & SbxValue::SaveData, we need to ensure that a 
Currency val written from the old Currency implementation can be read 
with the new implementation and vice-versa ( looking at the code it 
seems that it should work ). Hmmm but thinking about this more I 
really don't see why this is necessary, I can't think of why a types 
value would be saved with this this code,
I still haven't figured out if this code is really used or not, 
currently erring on the side of caution I created a new stream operator 
for SvStream and additionally tweaked the handling for Currency and the 
SbxSALINT64 and SbxSALUINT64 from you orig patch, it seems there were 
some bugs with the orig patch. Currently I don't see an easy way of 
testing the changes ( so I have coded them untested ) so I hope that 
maybe another set of eyes might help. Please feel free to do that ( and 
I'll try and ask on the list for some help too )
It would be great if you could look over my changes and see if they 
are ok, if you are happy with the change and if we can satisfy 
ourselves that LoadData/SaveData item above is not an issue then we 
could probably commit this to the master, then we can look further 
into some of the ifdefed out stuff, how does that sound?
did you look at the changes I committed to the branch ? ( of course 
there are some more changes to look at now :-) ) I'd be interested in 
your thoughts as I would like to get this stuff into master asap


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

Re: [Libreoffice] Need review of the ooo/OOO330_m18 changes

[Noel Power]

On 14/12/10 20:19, Jan Holesovsky wrote:

Hi,

I wanted to merge the m18 changes today, but looking at the diff, it is
not that trivial, and I don't feel like the right person reviewing it
according to our post-RC criteria; so I'd like to ask you to review it.
Best if you can merge/cherry-pick the changes too, if OK :-)  The
following repositories are affected:

filters
impress
libs-core
libs-extern-sys
libs-gui
writer

I pushed the changes from libs-core and libs-gui, relates to dba33m ( 
connectivity changes where we don't typically make and changes ) and 
impress205 which seemed fairly self contained.


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

Re: [Libreoffice] [PATCH] Basic Currency Issues #i31001# to #i107277#

[Noel Power]

On 15/12/10 10:04, Noel Power wrote:

Hi John,
On 06/12/10 17:58, Noel Power wrote:

 Hi John,

[...]
in  sbxToUnoValueImpl ( convert to uno without known target class ) 
in basic/source/classes/sbunoobj.cxx previously an Double or Float 
basic value ( within the range of an int ) was transfered as a the 
smallest type e.g. sal_Int8, sal_Int16 or sal_Int32 values greater 
were transferred as Float or Double
I removed the '#if MAYBEFUTURE'  I put around the code above. After 
some poking around and also looking at the dev guide it seems this 
code is to handle the case where the expected type ( on the uno side ) 
is unknown, in this case the value is down cast to the smallest ( 
non-internal ) basic type which is then converted to an uno type ( or 
at least that's my current take on the intention )


in basic/source/runtime/methods1.cxx you have added incomplete 
read/write support for types SbxSALINT64 & SbxSALUINT64,  lets not 
add support to persist a new type incompletely ( to easy to forget 
about ) so I've  #ifdefed it out ( with #if IMPLEMENTATION_READY )
I'm not sure it is needed but I enabled this ( and created a new 
stream operator to handle the 64 bit type )


I rewrote the ImpCurrencyToString, I took note of your suggestions 
re. optimizations whilst trying to point you to some existing 
functionality available from the String classes as its better to 
avoid generating even more number to string code. Somewhere between 
trying to write down some explanations and suggestions I'm afraid I 
found myself actually rewriting this and then I found mistakes in my 
'new' implementation... couldn't help myself. Anyway hopefully some 
of the code there shows some other possibilities to achieve the same 
thing reusing some of the existing LO classes. ( I have no doubt that 
there probably is some bugs in my implementation too ). Additionally 
I am still very uncertain about   ImpStringToCurrency() changes to 
use Locale specific separators. Also not sure about the local 
seperator logic, the heuristics used I think might give an undesired 
result under some circumstances. I don't want to rule it out but for 
the moment I would feel happier with the locale specific part being 
#ifdefed out ( so I did that with some more   #if MAYBEFUTURE blocks ).
I also rewrote the ImpStringToCurrency a little, actually more like I 
combined yours and the old implementation a little, mostly I just 
removed the home grown parser in favour of using the existing 
string->number conversions in OUString. Like the ImpCurrencyToString I 
have enclosed the locale specific functionality in a '#if MAYBEFUTURE' 
block. I removed the acceptance of blank characters in the middle of a 
string as VBA regards this as an error and the old implementation just 
truncated the number ( e.g. "12345 6" would end up as 12345 ), I raise 
an error in this case now. Hopefully if this breaks existing macros it 
will only break code already deeply in error ( from mis-interpreting 
the string/number  )  The other reason for removing the parser is 
if/when we do apply some locale specific separator processing I would 
love to reuse/share whatever calc currently uses.



SbxValue::LoadData & SbxValue::SaveData, we need to ensure that a 
Currency val written from the old Currency implementation can be read 
with the new implementation and vice-versa ( looking at the code it 
seems that it should work ). Hmmm but thinking about this more I 
really don't see why this is necessary, I can't think of why a types 
value would be saved with this this code,
I still haven't figured out if this code is really used or not, 
currently erring on the side of caution I created a new stream 
operator for SvStream and additionally tweaked the handling for 
Currency and the SbxSALINT64 and SbxSALUINT64 from you orig patch, it 
seems there were some bugs with the orig patch. Currently I don't see 
an easy way of testing the changes ( so I have coded them untested ) 
so I hope that maybe another set of eyes might help. Please feel free 
to do that ( and I'll try and ask on the list for some help too )
It would be great if you could look over my changes and see if they 
are ok, if you are happy with the change and if we can satisfy 
ourselves that LoadData/SaveData item above is not an issue then we 
could probably commit this to the master, then we can look further 
into some of the ifdefed out stuff, how does that sound?
did you look at the changes I committed to the branch ? ( of course 
there are some more changes to look at now :-) ) I'd be interested in 
your thoughts as I would like to get this stuff into master asap
pushed to master, hmmm I noticed running the test document the results 
are slightly worse than previously, I need to investigate how I seem to 
have broken this. I was sure that I retested ( all but the last changes 

Re: [Libreoffice] [PATCH] Basic Currency Issues #i31001# to #i107277#

[Noel Power]

On 22/12/10 10:13, Tor Lillqvist wrote:

It seems that this patch is now in master, and it causes compilation errors on 
Windows (slightly edited compiler messages for brevity):

sbxdec.cxx(735) : error C2065: 'n' : undeclared identifier
sbxdec.cxx(749) : error C2039: 'pULong64' : is not a member of 'SbxValues'
sbxdec.cxx(749) : error C3861: 'ImpDoubleToUINT64': identifier not found
sbxdec.cxx(756) : error C2039: 'pLong64' : is not a member of 'SbxValues'
sbxdec.cxx(756) : error C3861: 'ImpDoubleToINT64': identifier not found
sbxdec.cxx(759) : error C2196: case value '16390' already used
sbxdec.cxx(763) : error C2039: 'pLong64' : is not a member of 'SbxValues'I
I didn't even realise there was any windows specific code, I think I 
have an old windows build ( maybe I can just update the relevant parts 
)  let me see


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

Re: [Libreoffice] Build errors

[Noel Power]

On 22/12/10 02:23, Balachandran Sivakumar wrote:

call of overloaded 'append(char)' is ambiguous
Hmm I never got that error but happily it seems Joseph Powers commited a 
fix for that, try pull and rebuild.

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

Re: [Libreoffice] [PATCH] Remove DECLARE_LIST( EditList, AppWIn*)

[Noel Power]

Hi Joseph
On -10/01/37 20:59, Joseph Powers wrote:

The attached patch compiles and stands up to my limited testing; however, it's a 
large patch and touches a  lot of sensitive code so I want someone with better 
knowledge of the Basic Macro Editor  environment to review&  test it before I 
try pushing it.

Thanks,
Joe P.

I wouldn't claim to have much knowledge of the Basic Macro editor but I 
suppose at least I have touched it in the past ;-)
Firstly congratulations on this is great work, getting rid of those 
horrific macro lists and replacing with something more modern surely 
will make things easier for new developers to understand.  As far as I 
can see the changes as they stand shouldn't cause any problems and could 
be committed.
My only comment would be that mostly  the opportunity to simplify the 
code using the power of the vector has not being taken advantage of 
which is a pity ( and would be great to fix here ) e.g.


@@ -627,17 +627,15 @@ void BasicFrame::LoadIniFile()
 if ( pBasic )
 pBasic->LoadIniFile();

-for ( i = 0 ; i < pList->Count() ; i++ )
-pList->GetObject( i )->LoadIniFile();
+for ( i = 0 ; i < pList->size() ; i++ )
+pList->at( i )->LoadIniFile();
 }

could be much clearly expressed using iterators ( and no need to check 
the size every loop iteration either )

e.g. something like

EditList::iterator it = pList->end()
for ( EditList::iterator it = pList->begin(); it != it_end; ++it )
*it->LoadIniFile();

similarly really ugly index manipulation e.g.

@@ -777,11 +775,10 @@ void BasicFrame::Resize()


 // Resize possibly maximized window
-ULONG i;
-for( i = pList->Count(); i > 0 ; i-- )
+for( size_t i = pList->size(); i > 0 ; i-- )
 {
-if ( pList->GetObject( i-1 )->GetWinState() == TT_WIN_STATE_MAX )
-pList->GetObject( i-1 )->Maximize();
+if ( pList->at( i-1 )->GetWinState() == TT_WIN_STATE_MAX )
+pList->at( i-1 )->Maximize();
 }
 }

could be simplified and avoided by again making use of the power of the 
vector


EditList::iterator::reverse_iterator rit_end = pList->rend();
for ( EditList::iterator::reverse_iterator rit=pList->rbegin() ; 
rit != rit_end; ++rit )

{
if ( *rit->GetWinState() == TT_WIN_STATE_MAX )
*rit->Maximize();
}

nearly all of the hunks in the patch could be modified similarly I think.
note: my crappy pseudo code bits above are not compiled or tested but 
you get what I mean I am sure


thanks again

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

[Libreoffice] [REVIEW] request for review for patch for fdo#32742# regression

[Noel Power]

Hi there

fix for fdo#32742#, its a simple fix ( but was a bugger to find ) imo is 
riskless etc.


http://cgit.freedesktop.org/libreoffice/base/commit/?id=0afe2266016b03f6e11008463042c7daacead0e1
is ripe for signoff and cherrypicking, be grateful if someone could help 
with that


thanks,

Noel



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

[Libreoffice] [REVIEW]

[Noel Power]

I'd like to include this fix ( which fixes corruption of formfield 
checkbox related data in binary format on export ) in libreoffice-3-3


http://cgit.freedesktop.org/libreoffice/writer/commit/?id=af4d6062b7e676ed56dc84927fb99b79aa03a7b5

is available for cherrypick/signoff

thanks,
Noel

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

Re: [Libreoffice] [REVIEW] (Offensive Word Found In Message)

[Noel Power]

On 12/01/11 12:25, Michael Meeks wrote:

On Wed, 2011-01-12 at 06:17 -0600, Norbert Thiebaud wrote:

is available for cherrypick/signoff

It looks fine to me.

Merged to libreoffice-3-3 and libreoffice-3-3-0, and re-working in
master to suck less too ;-)

Thanks,

Michael.

I think this is the wrong thread, this is a different review request ( 
from the single s_nCount one ) and is to do with export filter 
functionality.
But.. nice to hear that this "global variable/fixed compile-time" thingy 
is being reworked ( to be honest though this pattern of static 
initialisation doesn't bug me, maybe I have become immune )


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

Re: [Libreoffice] [REVIEW] request for review for patch for fdo#32742# regression

[Noel Power]

On 12/01/11 18:03, Norbert Thiebaud wrote:

On Wed, Jan 12, 2011 at 10:35 AM, Octavio Alvarez
  wrote:

On Wed, 12 Jan 2011 03:18:01 -0800, Noel Power  wrote:


Hi there

fix for fdo#32742#, its a simple fix ( but was a bugger to find ) imo is
riskless etc.


http://cgit.freedesktop.org/libreoffice/base/commit/?id=0afe2266016b03f6e11008463042c7daacead0e1
is ripe for signoff and cherrypicking, be grateful if someone could help
with that

Is suggest to make ::std::sort on the next line use s_nCount also and
prevent a useless division operation.

Actually the 'useless' division' is resolved at compile time. so it is free.
on the other hand passing s_nCount instead of a constant _could_
deprive the template code of some possible optimizations.
(I don't know if std::sort is smart enough to take avantage of it..
but it could, and eventually it will)

Norbert

the other thing to remember is that this is some static initialisation, 
it is done just once, so even thinking about optimising that would seem 
to be overkill :-)

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

Re: [Libreoffice] (un)localized MSI installer [REVIEW REQUEST]

[Noel Power]

On 13/01/11 21:18, Tor Lillqvist wrote:

So the problem must be in setup.exe, in some cases it fails to understand
that it should use a transform. That is good, because it means we can debug it.

[...]

The symptom of the bug was that only the first 20 or so languages in the 
setup.ini file were recognized. German happens to be among those, which 
explains why it worked for Thalion72. Hungarian, Swedish and Portuguese 
(Brazil) are not, so they didn't work. In a test build with just a few 
languages, they all worked...

Please review the patch. Three approvals are needed to get this into the 3.3 
installer.

--tml



looks sensible to me :-)

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

Re: [Libreoffice] [PATCH] unopkg crasher on SLED11-SP1 (bnc#655912)

[Noel Power]

On 13/01/11 20:25, Petr Mladek wrote:

Hi,

could you please review the attached patch for libreoffice-3-3 branch?

It looks like a compiler bug. I see it only on SLED11 with one
particular gcc version. So, the solution is a kind of workaround but
it should be fine in general. See the patch for more details.

patch looks fine to me ( weird problem of course ) :-)
+1

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

Re: [Libreoffice] [REVIEW] More reviews for fdo#33108 fix please

[Noel Power]

On 17/01/11 17:51, Thorsten Behrens wrote:

Hi,

Lubos fixed an ugly crasher in vcl via fdo#33108 - I've stripped the
patch down to the absolute minimum, and would be ok for taking that
for -3-3-0 - any further reviews, please?
not being familiar with the cairo and vcl stuff but assuming the patch 
on master is correct then this version for '3-3-0'  is imo also correct 
so +1 ( but a pity of course the cleanup of the awful 'void*' and others 
is lost )


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

[Libreoffice] Fwd: [Libreoffice-bugs] [REVIEW] fdo#33237

[Noel Power]

 sent to the wrong list...

Noel

 Original Message 
Subject:[Libreoffice-bugs] [REVIEW] fdo#33237
Date:   Tue, 18 Jan 2011 13:02:38 +
From:   Noel Power 
To: libreoffice-b...@lists.freedesktop.org



I'd like to add this for the libreoffice-3.3 branch ( so at least one
approval needed )

see
http://cgit.freedesktop.org/libreoffice/filters/commit/?id=c764fd6bbe7fc04f0518d26e146e62671544b053
and
http://cgit.freedesktop.org/libreoffice/filters/commit/?id=60d77debbd521b95acda9c61a1dbdb9209914b67

thanks,
Noel
___
Libreoffice-bugs mailing list
libreoffice-b...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs

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

Re: [Libreoffice] NEW [PATCH] Unix 64 bits property type / default value mismatch

[Noel Power]

On 21/01/11 06:59, Lionel Elie Mamane wrote:

On Mon, Jan 17, 2011 at 04:58:28PM +, Caolán McNamara wrote:

On Mon, 2011-01-17 at 15:18 +0100, Lionel Elie Mamane wrote:

(...) we have to make these three places
agree. As to whether it is in the direction of "C/C++ long" everywhere
or in the direction of "uno long == sal_Int32" everywhere, (..)
[no] opinion.

Well, we have to assume from that LoadTableWindowData that the
values may have been cached on disk somewhere, and have been
assigned a data type of sal_Int32 for the purposes of file
storage. So I say we use sal_Int32 everywhere for those four
"long"s.

OK, here's the patch that changes the setters to sal_Int32 then.
minor minor nit-pick, wouldn't it be better to use static_cast here ( at 
the very least that helps to grep for casts ) - and yes, I am guilty of 
not doing this too ( but I try to remember )

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

Re: [Libreoffice] [REVIEW] (Offensive Word Found In Message)

[Noel Power]

On 14/01/11 12:17, Caolán McNamara wrote:

On Wed, 2011-01-12 at 12:25 +, Michael Meeks wrote:

On Wed, 2011-01-12 at 06:17 -0600, Norbert Thiebaud wrote:

is available for cherrypick/signoff

It looks fine to me.

Merged to libreoffice-3-3 and libreoffice-3-3-0, and re-working in
master to suck less too ;-)

Side note, FFDATA thing is also usable for pulling out the "real" URL
for hyperlink fields. There is a bug out there where HYPERLINK fields
are imported incorectly, because I think word actually ignores the
actual field text of the URL and use the FFDATA entry instead when it
exists.
Hmm missed this message in the email forest. Interesting, do you have 
more info on that? it seems lately I have stumbled into this code by 
accident a couple of time, if you have a bug # I might look further then 
next time. I presume though you don't have any objections to this though 
right?

C.

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


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

[Libreoffice] [REVIEW] rudimentary handling for odd/even section breaks

[Noel Power]

fix for https://bugzilla.novell.com/show_bug.cgi?id=659361
fix here
http://cgit.freedesktop.org/libreoffice/writer/commit/?id=da0006a1b5d7be8411d6fbb0190c1c4d44809458

Caolan, any thoughts?

be nice for 3.3.1 too !

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

Re: [Libreoffice] [REVIEW] fix for fontwork toolbar

[Noel Power]

On 04/02/11 10:04, Cedric Bosdonnat wrote:

Hi all,

Could anyone review the fix for this quite annoying bug?
https://bugs.freedesktop.org/show_bug.cgi?id=32872

The fix has been pushed to master as commit:
http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=05ef821abdff1ec4712cd4b054a21e4180b2298f

It would be nice to have that in 3.3.1.

Regards,


+1 for 3.3, looks good to me ( once it was pointed out to me what the 
actually difference was )


Noel

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

[Libreoffice] [REVIEW] fdo#30879

[Noel Power]

This fix reverts a libreoffice specific change which introduced a 
regression. please review for libreoffice-3.3 branch

fix ( on master ) is here
http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=6eb4e55b0a61f793f635a8038895cb2e452dc280

thanks,

Noel

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

Re: [Libreoffice] Review request: http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=7bc42b864b22fd8e0b159dfec212abaf989ff9aa

[Noel Power]

Hi Tor
On 04/02/11 15:34, Tor Lillqvist wrote:

I would like to cherry-pick that to libreoffice-3-3. (Otherwise it can't be 
built on Windows with a recent DirectX SDK.)



looks ok to me, I would say go for it

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

[Libreoffice] [REVIEW] patch for crasher fdo#37045

[Noel Power]

please see https://bugs.freedesktop.org/show_bug.cgi?id=37045 and the 
patch attached here ( https://bugs.freedesktop.org/attachment.cgi?id=46565 )


if someone would review / cherrypick etc. for libreoffice-3.4 branch 
that would be nice


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

Re: [Libreoffice] [Patch]Clean up XDatabaseRanges in VBA code

[Noel Power]

Hi MArkus
On 11/05/11 00:32, Markus Mohrhard wrote:

Hello Noel,

with my last patch I forgot to remove all occurences of 
XDatabaseRanges in the vba code. I don't think that there is any use 
for XDatabaseRanges in the vba code, because excel only supports one 
db data.


Regards,
Markusl

looks fine, go ahead and commit it, thanks again

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

[Libreoffice] [REVIEW] fix for crasher fdo#37103

[Noel Power]

Hi All,

please see https://bugs.freedesktop.org/show_bug.cgi?id=37103
and if someone could review and cherry-pick the patch 
http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=17e8ee1d7b9b4464af73657679df7b0bbdffa028 
from master, that would be great


thanks,

Noel


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

[Libreoffice] [REVIEW] fix for fdo#35139

[Noel Power]

Hi,

Please have a find a fix for 
https://bugs.freedesktop.org/show_bug.cgi?id=35139 here 
https://bugs.freedesktop.org/attachment.cgi?id=46611  I'd like to apply 
that to libreoffice 3.4


thanks

Noel

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

Re: [Libreoffice] [REVIEW] fix for #36915 and #36916

[Noel Power]

I mentioned the wrong bug, the patch fixes the above

On 11/05/11 21:46, Noel Power wrote:

Hi,

Please have a find a fix for 
https://bugs.freedesktop.org/show_bug.cgi?id=35139 here 
https://bugs.freedesktop.org/attachment.cgi?id=46611  I'd like to 
apply that to libreoffice 3.4


thanks

Noel

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


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

Re: [Libreoffice] [REVIEW] fix for #36915 and #36916

[Noel Power]

please see updated patch

Noel


diff --git scripting/util/provider/beanshell/makefile.mk scripting/util/provider/beanshell/makefile.mk
index cbb0c40..1f70fcb 100755
--- scripting/util/provider/beanshell/makefile.mk
+++ scripting/util/provider/beanshell/makefile.mk
@@ -36,6 +36,7 @@ EXTENSIONNAME:=ScriptProviderForBeanShell
 EXTENSION_ZIPNAME:=script-provider-for-beanshell
 
 COMPONENT_JARFILES=$(EXTENSIONDIR)$/$(EXTENSIONNAME).jar
+EXTENSION_PACKDEPS=$(SOLARBINDIR)$/bsh.jar
 
 .IF "$(ENABLE_SCRIPTING_BEANSHELL)" != "YES"
 @all:
@@ -46,6 +47,12 @@ COMPONENT_JARFILES=$(EXTENSIONDIR)$/$(EXTENSIONNAME).jar
 .INCLUDE : target.mk
 .INCLUDE : extension_post.mk
 
+ALLTAR : $(EXTENSIONDIR)$/bsh.jar
+
+$(EXTENSIONDIR)$/bsh.jar : $(SOLARBINDIR)$/bsh.jar
+@@-$(MKDIRHIER) $(@:d)
+$(COMMAND_ECHO)$(COPY) $< $@
+
 .ELSE
 
 .INCLUDE : target.mk
diff --git scripting/util/provider/javascript/manifest.xml scripting/util/provider/javascript/manifest.xml
index 4c61747..d052ad8 100755
--- scripting/util/provider/javascript/manifest.xml
+++ scripting/util/provider/javascript/manifest.xml
@@ -3,8 +3,5 @@
 http://openoffice.org/2001/manifest";>
   
-  
 
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [REVIEW] Fix for fdo#36667 for 3.4

[Noel Power]

On 11/05/11 16:36, Cedric Bosdonnat wrote:

Hi all,

Attached is a patch fixing the sw table formulas update in 3.4. Could
someone review and sign it off? It's a partial revert from

http://cgit.freedesktop.org/libreoffice/writer/commit/?id=2b9726405495529d64e7ab8537ea30119f607d1e

The test in the if, was actually setting pNewVal... which was never set
and causing troubles in some (potentially loads of) cases.

ah, nasty ( and not obvious ) ... signed off

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

[Libreoffice] [REVIEW] patch for #36684

[Noel Power]

Hi,
can someone please review ( and cherrypick ) to libreoffice3.4 the 
following patch

http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=ea29c4f0d289890fcdb2f12380aa1ccf3d1028c9
from master
thanks,
Noel
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

[Libreoffice] problems with component registration

[Noel Power]

Hi David,

It seems that 
http://cgit.freedesktop.org/libreoffice/libs-gui/commit/?id=f9496177a4c942f2acc39a978a3cd65689f14d8d 
is causing some problems with component registration with at least basic 
on master. I have a temporary fix for this here 
http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=1aa0150844d37e28cf0b420cbc60a96ad478edbe. 
The problem stems from the fact the legacy code (now) needs 
OModule.registerClient to be  called. However that call is never made. 
I'm not familiar with these helper classes so I am hoping you might know 
how the register/revoke client stuff should work ( and who/when/where 
should call what )
My main worry though is that perhaps this changes breaks more than 
basic. so... in the mean time it might be worth considering backing out 
that change



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

Re: [Libreoffice] LO with clean configuration unusable

[Noel Power]

On 17/05/11 15:13, Lubos Lunak wrote:

  Hello,

  current master, with ~/.libreoffice removed, fails on the first startup,
showing a dialog about not being able to lauch the application. Subsequent
launches are successful, but LO is unable to load or save any document (I
assume the first run resulted in a corrupted internal database about
plugins).

  The commit responsible for this problem is
f9496177a4c942f2acc39a978a3cd65689f14d8d in libs-gui. I don't understand the
code, but apparently the new constraints introduced by the commit are not met
by code using the class.

I reverted that commit and my associated hack 
http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=1aa0150844d37e28cf0b420cbc60a96ad478edbe 
seems the problem is more widespread than just basic. Also not necessary 
for a clean install, e.g. docx filters can't be instantiated



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

Re: [Libreoffice] [REVIEW] [PUSHED]fix for fdo#37277

[Noel Power]

On 17/05/11 15:37, Cedric Bosdonnat wrote:

m_bResetUndo = sal_False


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

[Libreoffice] REVIEW for fix#37211

[Noel Power]

Hi,

following patch fixes regressions reported in 
https://bugs.freedesktop.org/show_bug.cgi?id=37211 & 
https://bugs.freedesktop.org/show_bug.cgi?id=37170


https://bugs.freedesktop.org/attachment.cgi?id=46831

it would be great to get the 1 review for libreoffice-3-4  ( not 
critical so no point pushing for the release branch I think )


thanks,

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

Re: [Libreoffice] REVIEW for fix#37211

[Noel Power]

On 17/05/11 20:13, Clio wrote:

17.05.2011 12:47, Noel Power пишет:


( not critical
so no point pushing for the release branch I think )


If it is not fixed in 3.4.0, you can't Select All neither with the menu
Select All from the menu has been broken forever, see 
http://openoffice.org/bugzilla/show_bug.cgi?id=5048 (  about to 
celebrate it's 7th birthday  )

nor with Ctrl A.
you can temporarily use cntrl-shift-A  which is an undocumented 
hardcoded accelerator that this patch will remove

And Delete key not working would be very annoing too.
I don't believe that not being able to use the delete key is a major 
impediment, you can always use backspace.
In anycase this should be fixed at the latest in 3.4.1, if we get the 
right amount of reviewers it will appear in 3.4.0.


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

Re: [Libreoffice] [REVIEW] Fix for fdo#37226

[Noel Power]

On 17/05/11 17:04, Kohei Yoshida wrote:

Hi there,

The attached patch fixes the crasher reported in the following bug:

https://bugs.freedesktop.org/show_bug.cgi?id=37226

and also several other usage of standard algorithms which I believe is
incorrect, since the existing code passes the result of an evaluation
which is always boolean (unless boost::bind overloads those operators to
return something else, which I doubt it does).
I don't know much about boost, have seen the bind stuff peppered here 
and there in the code and generally liked the look of it, anyway your 
question prompted me to try and understand more, the following link 
http://www.boost.org/doc/libs/1_46_1/libs/bind/bind.html#operators seems 
to indicate that it is the operator is passed to the standard algorithm 
and not the result ( but maybe I misunderstood, this stuff is still 
unfamiliar to me )

   There were several cases
where such evaluation result was passed to standard algorithms, one of
which caused the crash.
I tried your patch ( but with the bind bits left in ) and it seemed to 
work find for me ( especially wrt to the operator stuff above ) attached 
is the patch I used


Noel
diff --git a/sc/source/ui/docshell/autostyl.cxx b/sc/source/ui/docshell/autostyl.cxx
index d05676e..d1fa26f 100644
--- a/sc/source/ui/docshell/autostyl.cxx
+++ b/sc/source/ui/docshell/autostyl.cxx
@@ -108,8 +108,10 @@ void ScAutoStyleList::AddEntry( sal_uLong nTimeout, const ScRange& rRange, const
 aTimer.Stop();
 sal_uLong nNow = TimeNow();
 
-aEntries.erase(std::remove_if(aEntries.begin(),aEntries.end(),
-  boost::bind(&ScAutoStyleData::aRange,_1) == rRange));
+::boost::ptr_vector::iterator itr = std::find_if(aEntries.begin(),aEntries.end(), boost::bind(&ScAutoStyleData::aRange,_1) == rRange);
+
+if (itr != aEntries.end())
+aEntries.erase(itr);
 
 //	Timeouts von allen Eintraegen anpassen
 
@@ -145,19 +147,19 @@ void ScAutoStyleList::AdjustEntries( sal_uLong nDiff )	// Millisekunden
 
 void ScAutoStyleList::ExecuteEntries()
 {
-boost::ptr_vector::iterator iter;
-for (iter = aEntries.begin(); iter != aEntries.end();)
+// Execute and remove all items with timeout == 0 from the begin position
+// until the first item with non-zero timeout value.
+::boost::ptr_vector::iterator itr = aEntries.begin(), itrEnd = aEntries.end();
+for (; itr != itrEnd; ++itr)
 {
-if (!iter->nTimeout)
-{
-pDocSh->DoAutoStyle(iter->aRange,iter->aStyle);
-iter = aEntries.erase(iter);
-}
-else
-{
-++iter;
-}
+if (itr->nTimeout)
+break;
+
+pDocSh->DoAutoStyle(itr->aRange, itr->aStyle);
 }
+// At this point itr should be on the first item with non-zero timeout, or
+// the end position in case all items have timeout == 0.
+aEntries.erase(aEntries.begin(), itr);
 }
 
 void ScAutoStyleList::ExecuteAllNow()
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [REVIEW] Fix for fdo#37226

[Noel Power]

Hi Kohei
On 18/05/11 12:52, Kohei Yoshida wrote:

On Wed, 2011-05-18 at 12:08 +0100, Noel Power wrote:

[...]

So, we (Rafael, Norbert, Joseph and I) talked about this last night on
IRC, and one question raised was whether or not boost::bind allows
pointers to data members rather than member functions.  The
documentation clearly mentions of members functions, but is a bit
ambiguous with data members.
aha, you know I didn't even notice that it was dealing with member 
variables instead of member functions :-

  In fact, the standard function adapters
that boost::bind wraps seems to only support member functions which made
me believe that the use of bind with data members is not allowed and
that if it works with some compilers it's just by accident and is not
really guaranteed with different compilers.

Having said that, if boost::bind is guaranteed to work with data
members, then I prefer keeping that piece in&  prefer your patch over
mine.  Otherwise, I'd rather we don't use it with data members.
the way I see it is that member function pointers and member data 
pointers ( I never even heard of those before ) are different, just some 
experimentation with some simple test code shows you can't exchange 
them. Since boost is heavily templatized ( and hence quite type-safe ) 
it would seem to be certain that there is some overloading in the 
boost::bind implementation to ensure that pointer to member mojo is 
supported. In otherwords I would expect compile time failures rather 
than runtime ones. Of course you are right to ask do all compilers 
support this, but I guess since we know of no failures sofar we can say 
for certain that the compilers and platforms we care about cater for this.


Anyway +1 from me regardless of which version of the patch ( really 
referring the patch I attached as *mine* is rich ;-)  I only omitted 
some changes you did )


Noel

p.s. I tried to have a quick look at the boost headers to see if I could 
confirm any of my suspicions... the feeling of getting my brains sucked 
out of my head was too much and I had to look away ( that stuff is far 
to complicated for me )

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

Re: [Libreoffice] [PUSHED] [REVIEW] Fix for fdo#37226

[Noel Power]

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

Re: [Libreoffice] [PUSHED[ [REVIEW] Fix for fdo#37226

[Noel Power]

On 18/05/11 15:45, Kohei Yoshida wrote:

I accidentally pushed mine to the -3-4
branch already when I signed off and pushed Rafael's patch (since it was
already committed locally and pushing Rafael's also pushed mine with
it).;-)

So, let me just use that as my excuse for picking my version this time
around.;-)

marking as pushed then
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

[Libreoffice] [REVIEW] fix for fod#36552

[Noel Power]

Hi
I have a fix for https://bugs.freedesktop.org/show_bug.cgi?id=36552
I'd like to get it into the 3.4 branch if 
possiblehttp://cgit.freedesktop.org/libreoffice/writer/commit/?id=8f1fe65d4fab09515b0b67319fef67c88533b2e0


thanks,

Noel


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

Re: [Libreoffice] [REVIEW] fix for fod#36552

[Noel Power]

On 18/05/11 16:48, Cedric Bosdonnat wrote:

Hi Noel,

The patch looks good to me. Still two other reviews to get it in 3.4.0
thanks, pushed to the libreoffice3-4 branch, of course if I get 2 more 
acks I'll push it to the 3.4.0 branch


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

Re: [Libreoffice] [REVIEW] fix for fdo#34997

[Noel Power]

Hi Cedric
On 18/05/11 17:09, Cedric Bosdonnat wrote:

Hi all,

Could you review the attached patch (sign it off, and so on)? It fixes a
crash when reading some malformed ODT files. It would be great to have
that one in 3.4.

looks fine to me, you got 1 signoff

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

Re: [Libreoffice] [PUSHED] [REVIEW] fix for fod#36552

[Noel Power]

On 18/05/11 17:05, Noel Power wrote:

On 18/05/11 16:48, Cedric Bosdonnat wrote:

Hi Noel,

The patch looks good to me. Still two other reviews to get it in 3.4.0
thanks, pushed to the libreoffice3-4 branch, of course if I get 2 more 
acks I'll push it to the 3.4.0 branch

pushed now to the libreoffice-3-4-0 branch, thanks for the reviews everyone
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

[Libreoffice] [REVIEW] fix for https://bugs.freedesktop.org/show_bug.cgi?id=37367

[Noel Power]

Hi
Please review this fix for a regression from 3.3.2,

patch is here ( ripe for picking ) 
http://cgit.freedesktop.org/libreoffice/filters/commit/?id=ac80fce47404c0843c372336ab700d62297f1647

on master, would be great to get into libreoffice3-4 branch at least
thanks,

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

Re: [Libreoffice] [REVIEW] Fix for fdo#37322: Calc binary import crasher

[Noel Power]

On 19/05/11 15:40, Kohei Yoshida wrote:

Looks good.  You have my sign-off.

Kohei


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

Re: [Libreoffice] [REVIEW] fix for https://bugs.freedesktop.org/show_bug.cgi?id=37367

[Noel Power]

On 19/05/11 17:01, Michael Meeks wrote:

On Thu, 2011-05-19 at 16:14 +0100, Noel Power wrote:

http://cgit.freedesktop.org/libreoffice/filters/commit/?id=ac80fce47404c0843c372336ab700d62297f1647

You have my +1 - and I've cherry-picked it to 3-4.

Thanks,

Michael.

I am sorry, I mistakenly included some (benign) debug code with the 
patch, Handler.cxx contained the real change, I reverted the patch ( on 
master and the branch ) and re-submitted it


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

Re: [Libreoffice] [REVIEW] trivial PPTX import crash fix

[Noel Power]

On 19/05/11 16:54, Michael Meeks wrote:

To avoid a NULL pointer de-reference, don't de-reference a null
pointer :-) I'd like to get this into 3.4.0

Thanks,

Michael.

looks sane to me +1
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [REVIEW] fix for fdo#36690

[Noel Power]

On 20/05/11 10:02, Michael Meeks wrote:


Weelll ... it seems that (for all its ugliness) the more common way of
doing this inside button.cxx's draw code (cf. ImplDrawPushButtonFrame)
seems to be this:

diff --git a/vcl/source/control/button.cxx b/vcl/source/control/button.cxx
index c77d14a..6932cdb 100644
--- a/vcl/source/control/button.cxx
+++ b/vcl/source/control/button.cxx
@@ -1522,7 +1522,7 @@ void PushButton::Draw( OutputDevice* pDev, const Point&  
rPos, const Size&  rSize,
  else
  aStyleSettings.SetFaceColor( 
GetSettings().GetStyleSettings().GetFaceColor() );
  aSettings.SetStyleSettings( aStyleSettings );
-pDev->SetSettings( aSettings );
+pDev->OutputDevice::SetSettings( aSettings );
  }
  pDev->SetTextFillColor();


I'd feel happier with that personally too, it also seems to fix the
bug.

Thanks !

Michael.

I prefer Michael's version, so +1 for that ( although I don't agree with 
the ugly comment, to me it shows the intentional bypassing of the 
dynamic binding )

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

Re: [Libreoffice] [REVIEW] fix for fdo#36690

[Noel Power]

On 20/05/11 10:43, Noel Power wrote:

On 20/05/11 10:02, Michael Meeks wrote:


I prefer Michael's version, so +1 for that ( although I don't agree 
with the ugly comment, to me it shows the intentional bypassing of the 
dynamic binding )

___

I committed that version to 3.4 branch, I'm sorry I forgot the --author :-(
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] minutes of tech. steering call ... (Offensive Word Found In Message)

[Noel Power]

On 20/05/11 08:27, Cedric Bosdonnat wrote:

Hi all,

Sorry, I couldn't join yesterday. I'm just wondering where the
reviewboard action item went? is that a dropped idea or does it still
deserves some action?
I meant to say something when this was originally floated but got 
distracted. I think the idea of those kinda tools are good, I have even 
used similar ( maybe even it was the the one ) tools previously for 
reviewing and found them really good. However I would say that I think 
these work best in a closed environment,  by closed I mean a tight group 
that are the reviewers that control the whole thing. The disadvantages 
for us I see is the need for a new list to subscribe to ( who needs yet 
another list ) or having to have an account in some webserver that hosts 
the tool(s)
Posting to the dev list to me remains in my view the most inclusive way 
of doing reviews, it's process light, everyone gets to see what 
happening, and most importantly ( imo ) all can learn from the reviews ( 
it's real easy to dip-in/out of any review thread as you wish )
I really wish to stress the 'learn' aspect because I think reviews are a 
key way for people to learn about the code ( no joke I think I learn 
something new every day on this list from the reviews that I read ) imho 
we should make it as easy for people to be exposed to the reviews as 
possible


just my 2 whatever

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

Re: [Libreoffice] Please, review the patch to baside2b.cxx on 17-Feb-2011

[Noel Power]

On 20/05/11 15:53, Clio wrote:

Please, see here, that the patch actually added 1 line:
http://svn.services.openoffice.org/opengrok/diff/DEV300_m106/basctl/source/basicide/baside2b.cxx?r2=%2FDEV300_m106%2Fbasctl%2Fsource%2Fbasicide%2Fbaside2b.cxx%40275666%3Ad11c441ed074&r1=%2FDEV300_m106%2Fbasctl%2Fsource%2Fbasicide%2Fbaside2b.cxx%40273924%3Ada255a63b325




Thanks for a clearer diff, the first one sent was too confusing to look at
So, you point to http://openoffice.org/bugzilla/show_bug.cgi?id=108119, 
to be honest I find the bug confusing ( the habit of mixing many 
different even if they seem related bugs in a single report is never a 
good idea ) worse still the information in there is conflicting as to 
which buttons work and which buttons don't.
But, it seems in summary Cntrl+Z didn't enable the toolbar redo, Andreas 
made some patch and it it didn't fix things completely, I hope that is 
an accurate.
Now we move onto bug https://bugs.freedesktop.org/show_bug.cgi?id=37405 
which you think is somehow related to the patch you mention. Have you 
tried without it ? because somehow I doubt that it could be responsible 
for it. So, as to whether it makes sense to just remove this ( 
especially as a point of urgency ) is debatable.

However as a crasher undoubtedly we need to look at this in more detail.

thanks,

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

Re: [Libreoffice] minutes of tech. steering call ... (Offensive Word Found In Message)

[Noel Power]

On 20/05/11 12:53, Norbert Thiebaud wrote:


I do agree. but then it should be possible to use a nice review-tools
and still have the generated review comment posted in the the ML

I mean, when someone add a patch to be review in the tool that can
generate a post to the ML with the patch (either inline if small or as
a link back to the reviewboard thing
doesn't that mean though that someone new who wants to actually 
participate and maybe is already intimidated by the code, the 
personalities ( dubious in some cases ;-) ) etc has only a readonly 
view in otherwords to make those first tentative steps to interact with 
a review has to go through the hoops and has to get into the reviewboard 
system/signon etc. To me currently it is just so much easier and use 
friendly ( and yes I know unfortunately without the nice tools :-( )


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

[Libreoffice] [REVIEW] fix for fdo#37370

[Noel Power]

Hi

Please review this fix 
https://bugs.freedesktop.org/attachment.cgi?id=47096 for 
https://bugs.freedesktop.org/show_bug.cgi?id=37370.It would have been 
great to get this into rc2 ( unfortunately I wasn't quick enough ) as 
this is an extremely annoying bug and makes identifying problems with 
basic code very difficult


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

Re: [Libreoffice] [REVIEW] fix for fdo#37370

[Noel Power]

On 24/05/11 11:36, Michael Meeks wrote:

Looks odd to me - this was not necessary in the past (at least
comparing with -3-3 and so on) - I suspect there is a deeper and more
interesting problem we're just band-aiding.
yes, resources from basic.srs were merged previously into ofa[en].res 
but also existed in sb[en].res, I would guess the problem stems from a 
partial removal of this duplication ( from m103 or whatever 3.4 is based 
on ) Not knowing where to start I spent a long time chasing this. In the 
end I found this is fixed in m106, knowing where to look at least it was 
easy to find the code to change ( the alternative was to modify the 
build to include basic.srs in ofa[en].res. When looking in m106 I 
noticed also some code doing a similar trick for svt[en].res, it's not 
clear if this change is also necessary for 3.4 ( but then again I didn't 
hear any problems in that area ) so either it's not an issue or we 
aren't getting the necessary errors to notice it.

Still - looks harmless, so I've committed to the -3-4 branch for you.

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

Re: [Libreoffice] [Review] patch for disappearing autofilter when inserting/deleting sheets

[Noel Power]

On 24/05/11 15:00, Kohei Yoshida wrote:


Looks good to me&  pushed to the -3-4 branch.  If two more people agree,
I'd like this pushed to the -3-4-0 branch as well.

Kohei


looks good to me too
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [REVIEW] Fix for fdo#37520

[Noel Power]

On 24/05/11 21:03, Kohei Yoshida wrote:

The attached patch fixes the crasher reported in

https://bugs.freedesktop.org/show_bug.cgi?id=37520

The crash occurs in ScRangeList::Join() where the array size is cached
prior to the for loop, but array may shrink during the loop, in which
case an out-of-bound array access may occur which in turn results in a
crash.  I'm actually surprised that this didn't cause a crash on
Linux.

Anyhow, the solution is to always dynamically check the size of the
array via size() method in each iteration, instead of caching it for
all iterations.

Review appreciated.  I'd like this to go into the -3-4-0 branch.  So I
need three sign-off's.

Kohei
the third and pushed to 3.4.0 ( not sure are we finished with that 
branch though )


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

Re: [Libreoffice] [PUSHED][PATCH] Replace List for std::map

[Noel Power]

On 26/05/11 15:42, Rafael Dominguez wrote:



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

Nice one, patch looked good to me,  pushed to master

thanks so much
Noel
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [PATCH] Fix out of bounds access in ImplEntryList

[Noel Power]

On 26/05/11 21:09, Rafael Dominguez wrote:
Fix possible oob when trying to erase an element. Please review and 
signoff for merging into 3.4.1




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

committed to master, List is still used in  libreoffice-3.4

thanks again

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

Re: [Libreoffice] [PUSHED] [PATCH] Fix out of bounds access in ImplEntryList

[Noel Power]

On 27/05/11 10:29, Noel Power wrote:



really mark as pushed
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [PUSHED] [PATCH] Remove commented out code

[Noel Power]

On 26/05/11 23:38, Luke Petrolekas wrote:

License LGPLv3+/MPL

More dead code.

Luke

Thank you for the nice patch, committed to master

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

Re: [Libreoffice] [PUSHED] [PATCH] Very small code cleanup.

[Noel Power]

On 26/05/11 23:48, Luke Petrolekas wrote:
No, this is not more dead code this time :). I noticed this and I'm 
pretty confident the code is sloppy. It's my first time deliberately 
touching the code, so sorry if it's a bit small.

nope, don't be sorry, useful cleanup, thanks again

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

Re: [Libreoffice] [PUSHED] [PATCH] Remove commented out code.

[Noel Power]

On 26/05/11 23:59, Luke Petrolekas wrote:

License LGPLv3+/MPL

thanks again,

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

[Libreoffice] [REVIEW] fix for fdo#37590

[Noel Power]

I'd like to get the following patch into 3.4 so 1 review needed

bug: https://bugs.freedesktop.org/show_bug.cgi?id=37590

patch: 
http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=ecbd796ee8157047b1738ac12c98a6ef4d3c18ff


hanks,
Noel


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

Re: [Libreoffice] [PATCH] fdo#34872

[Noel Power]

On 28/05/11 19:40, Rob Snelders wrote:

Hi,

Here is a really small patch to solve fdo#34872.
This patch is submitted under LGPLv3+/MPL-Licence.

looks good ( I suppose some risk that this might have some side affects 
for other fields that use this Expression field type, but ok, master is 
the place for such things. Thanks again, I haven't pushed it yet, with 
the m106 merge I don't have a working master yet ( mid-build ) I will 
push later, this message is just to tell you that you are not being 
forgotten !!


thanks again,

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

Re: [Libreoffice] [PUSHED] [PATCH] Code Cleanup in calc

[Noel Power]

On 29/05/11 05:23, Korrawit Pruegsanusak wrote:

Hello all,
The attached patch is to cleanup the code in calc.
Please feel free to comment.

thanks for the patch, nice job, pushed to master now

regards,

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

Re: [Libreoffice] [PUSHED] [PATCH] cppcheck cleanliness

[Noel Power]

On 29/05/11 05:42, Korrawit Pruegsanusak wrote:

Hello all,
The attached patch is to remove warning about "inefficient checking
for emptiness" from cppcheck.

thanks again for the nice patch,
pushed to master

Noel

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

Re: [Libreoffice] [PUSHED] [PATCH] fdo#34872

[Noel Power]

On 30/05/11 11:55, Noel Power wrote:

On 28/05/11 19:40, Rob Snelders wrote:

Hi,

Here is a really small patch to solve fdo#34872.
This patch is submitted under LGPLv3+/MPL-Licence.

looks good ( I suppose some risk that this might have some side 
affects for other fields that use this Expression field type, but ok, 
master is the place for such things. Thanks again, I haven't pushed it 
yet, with the m106 merge I don't have a working master yet ( mid-build 
) I will push later, this message is just to tell you that you are not 
being forgotten !!

ah, pushed anyway as it applies fine, I am too conservative sometimes

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

Re: [Libreoffice] [PUSHED] [PATCH] Code Cleanup in libs-gui

[Noel Power]

On 28/05/11 19:47, Korrawit Pruegsanusak wrote:

Hello all,
Although the diff may look frustrated, I've explained what I've done
in log message.
Please feel free to comment :-)

Nice patch, thanks again, I took the comment 'Clean background first 
because of plugins. ' from Christina, not sure if it makes any more 
sense for me but then again it makes more sense than a german comment :-)


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

Re: [Libreoffice] [PUSHED] [REVIEW] OOXML 2010 xlsx export

[Noel Power]

On 30/05/11 13:35, Katarina Machalkova wrote:

Hey there,

can some1 please review&  cherry-pick for 3.4 the following patch:

http://cgit.freedesktop.org/libreoffice/filters/commit/?id=6603c30f910b1fb5674a28988304dfcd14c11ecb

Seems that it somehow fell through the cracks and never made it to 3.4 branch
:-(  Sadly, saving spreadsheet docs with one of xlsx filters (Office Open XML
Spreadsheet) will now fail in 3.4.0 with generic "cannot write" error :-(
Let's fix at least 3.4.1 then ... sorry about that.


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

Re: [Libreoffice] [REVIEW] fix for fdo#37590

[Noel Power]

On 27/05/11 14:17, Noel Power wrote:

I'd like to get the following patch into 3.4 so 1 review needed

bug: https://bugs.freedesktop.org/show_bug.cgi?id=37590

patch: 
http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=ecbd796ee8157047b1738ac12c98a6ef4d3c18ff
Petr and I chatted briefly on IRC,  as he rightly pointed out most 
likely the intended change would be 
https://bugs.freedesktop.org/attachment.cgi?id=47337


thanks,

Noel




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

Re: [Libreoffice] [PATCH] fdo#32263 - Config file location

[Noel Power]

Hi
On 30/05/11 15:39, Takeshi Abe wrote:

Hi all,
(thanks to Noel, for helpful comments on its initial version and
suggesting to post to this list).
ahem, I seem to have gotten the wrong end of the stick about some of the 
patch ( or at least looking at it again I see that )

It seems to work well with master, but has one concern; the change
against common_brand.scp leads the path "$HOME/libreoffice" (not
"$HOME/.libreoffice") used in the fallback case,
the fallback though is pretty unlikely right ? ( only for a huge path ) 
My main gripe was that the scp change looked like it would impact 
windows too, I didn't look past the small amount of context in the patch 
so didn't realise that the value for the config changed here is indeed 
for unix only.

  which sounds
uncomfortable to some users including me.
Any thought

as above

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

Re: [Libreoffice] [PATCH] fdo#32263 - Config file location

[Noel Power]

On 31/05/11 01:41, Takeshi Abe wrote:

Hi Noel,

[...]

Well the change against common_brand.scp is surrounded with #ifdef, so will not
affect to WNT nor to MACOSX. Or am I misunderstanding something?
yes that's what I said ( probably not clearly enough ) e.g. I realised 
that "the config changed here is indeed for unix only" in other words I 
agree with you ;-)

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

Re: [Libreoffice] [PUSHED] [PATCH] Code cleanup in filters

[Noel Power]

On 30/05/11 18:18, Korrawit Pruegsanusak wrote:

Hello all,
Attached patch is a bit more code cleanup in filters.
Please feel free to comment.

Anyway, I've found some comments are unreadable, eg. in
/hwpfilter/source/hwpreader.cxx line ~4440. Should we also remove
them? or are they my encoding faults?

Best Regards,

it appears this has been already pushed to master, adding the PUSHED tag 
to subject line

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

Re: [Libreoffice] [PUSHED[ [REVIEWED] fix for fdo#37590

[Noel Power]

On 31/05/11 10:06, Michael Meeks wrote:

Hi Noel,

On Mon, 2011-05-30 at 20:40 +0100, Noel Power wrote:

Petr and I chatted briefly on IRC,  as he rightly pointed out most
likely the intended change would be
https://bugs.freedesktop.org/attachment.cgi?id=47337



as per the ack, pushed to 3.4
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [PUSHED] [PATCH] fdo#32263 - Config file location

[Noel Power]

On 31/05/11 09:25, Noel Power wrote:

On 31/05/11 01:41, Takeshi Abe wrote:

Hi Noel,

[...]
Well the change against common_brand.scp is surrounded with #ifdef, 
so will not

affect to WNT nor to MACOSX. Or am I misunderstanding something?
yes that's what I said ( probably not clearly enough ) e.g. I realised 
that "the config changed here is indeed for unix only" in other words 
I agree with you ;-)
As such ( and after getting my master build in order to verify it builds 
) I have pushed it to master, if there are any gross objections I guess 
people will should to revert it  :-)


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

Re: [Libreoffice] [PATCH] Duplicate code: join ImportFrom and InsertFrom

[Noel Power]

On 31/05/11 13:03, Chr. Rossmanith wrote:

Hi,

more duplicate code cleanup.

Class SfxObjectShell has two nearly identical methods: ImportFrom and 
InsertFrom. The latter has a few lines of code more, so I've removed 
InsertFrom (which was added to the code base later than ImportFrom), 
added a boolean parameter to ImportFrom and adjusted the few calls to 
those methods. And InsertFrom is not virtual like ImportFrom. Please 
review the attached patches. I'll commit them if I get an "ok".
looks good to me, please commit it to master ( or I will commit it later 
after I get a build /me unfortunately accidently did a make clean on his 
master build )


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

Re: [Libreoffice] [PUSHED[ [PATCH] fix for fdo#37761: Keyboard navigation broken in tools - options

[Noel Power]

On 31/05/11 18:03, André Schnabel wrote:

Hi,

we came across a minor accessability issue at the German discuss list. 
But the fix is so easy that even I could do it.


It's just to remove one line that breaks the keyboard navigation in 
the options dialog and has no sense at all. Christoph Noack confirmed 
tha change for UX at bugzilla.


Patch is contributed under LGPLv3+/MPL.

Nice patch, nice bug report, thanks a million, pushed to 3.4

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

Re: [Libreoffice] [PUSHED] Patch for Bug 34786: [EasyHack]Editing: Sentence movement function broken

[Noel Power]

On 01/06/11 06:34, jeffrey chang wrote:

Sorry here is the patch.

On Wed, Jun 1, 2011 at 1:18 PM, jeffrey chang > wrote:


Hi,

I am new to the LibreOffice community. I know that 34786 has been
posted for awhile, but no one seems to have claimed the bug and
the error was still reproducible in writer so I decided to take up
the bug as my first EasyHack. I seemed to have fixed the problem
of deleting to the beginning of the sentence by making a simple
change in wrtsh4.cxx (See Patch). After re-making sw and testing
in writer the problem seems fixed. The patch is on LO version
3.4.0.2 because one of the recent masters that I pulled was unstable

Great work, patch looks  fine to me and works as expected, pushed to 3.4 
branch. look forward to your next patch


thanks alot

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

Re: [Libreoffice] [PUSHED] Patch for Bug 34786: [EasyHack]Editing: Sentence movement function broken

[Noel Power]

On 01/06/11 12:05, Noel Power wrote:
On 01/06/11 06:34, jeffrey chang wrote: 

[...]
Sorry here is the patch.k, patch looks  fine to me and works as 
expected, pushed to 3.4 branch. look forward to your next patch


thanks alot

oh and can you please confirm the patch is contributed under LGPLv3+/MPL.?
thanks again,
Noel
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [PUSHED] [PATCH] restore Korean comments in hwpfilter (were corrupted to just "????" instead of being translated)

[Noel Power]

On 01/06/11 13:55, Christian Lohmaier wrote

Sure - I attached the patch for that as well :-)
(one time to have a list of strings that can be passed on to possible
translators, and of course for having it reviewed, in terms of does it
cause problems with compilers? Do all compilers deal with non-ascii in
comments (with UTF8, this should not be a problem, but who knows)
ok, I pushed it to master ( can't be worse than what there imo ), lets 
see if it causes any problems

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

Re: [Libreoffice] [PUSHED] [PATCH] fdo#32263 - Config file location

[Noel Power]

On 08/06/11 13:55, Takeshi Abe wrote:


OK, so do you mean libs-core/desktop/source/migration?, which functionality
I have not yet recognized clearly.
It would be great if anyone familiar with the code suggests a starting
point to dig into.
I *think* this ( see attached patch ) will fix it, not had a chance to 
test it yet
diff --git a/officecfg/registry/data/org/openoffice/Setup.xcu b/officecfg/registry/data/org/openoffice/Setup.xcu
index 37e489b..6f156d1 100644
--- a/officecfg/registry/data/org/openoffice/Setup.xcu
+++ b/officecfg/registry/data/org/openoffice/Setup.xcu
@@ -783,12 +783,12 @@
 
   
 
-  
+  
 
   10
 
 
-  Openofice.org 3=openoffice.org/3,OpenOffice.org 2=openoffice.org2,StarOffice 8=staroffice8,StarSuite 8=starsuite8
+  Libreoffice 3=libreoffice/3,Openofice.org 3=openoffice.org/3,OpenOffice.org 2=openoffice.org2,StarOffice 8=staroffice8,StarSuite 8=starsuite8
 
 
   
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice