I had done these a while ago, when I looked into extending loplugin:ostr to do
more automatic rewriting, and these were places where I needed to do something
manually, for one reason or another, because the automatic rewriting would not
pick it up correctly.
However, I got distracted, and a wholesale automatic rewrite would still run
into cases where an _ostr/_ustr instance from a library's .rodata would still be
referenced after the library has already been dlcose'd. So I never came around
to finishing all that.
But there appears to be renewed interest in (automatic) rewritings here now, so
it probably makes sense if I share this part of my work anyway.
Change-Id: I3da9d38398e4bca373cb0000a9d34b49a36ad58a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166792
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
Tested-by: Jenkins
Updates vbabutton.hxx to use pragma once
Change-Id: I55874083ad0cdbc08bedaa71fa7553acfefa3195
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164324
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
The ::Put methods at SfxItemSet had an extra WhichID
parameter that was not really documented, but I would
guess often asked why it exists: An extra WhichID, just
called 'nWhich' (which makes things NOT clearer). That
is 'strange' since the Item given to be put already
internally has a WhichID, so why a 2nd one?
If you were really interested and read all that code
(no, no comments on that anywhere) you might know
that this a kind of 'Target-WhichID' under which
the Item shall be put to the ItemSet. Since this
is unclear for most people it is even dangerous and
explains why so many code places just hand over the
WhichID requsted from the Item that already gets
handed over.
To make it short: I removed that. For the 19 places
where this was really needed I added a new method
besides ::Put called ::PutAsTargetWhich that takes that
extra WhichID (now called TargetWhich) and takes the
needed actions. These are quite some because that may
be combined with the bPassingOwnership flag, see new
SfxItemSet::PutImplAsTargetWhich method.
This makes usage of ItemSets/Items less dangerous. It
also simplifies and thus makes safer the central helpers
implCreateItemEntry/implCleanupItemEntry which have some
less cases to handle.
Debugged the failing UnitTests showed that there is
an incarnate Item != SfxVoidItem that causes problems.
I checked for errors in the change, but no luck.
Afterr some time I found out that a ::Clone
implementation caused the problem: These need to
also copy the WichID of the original, but the
SfxFrameItem failed to do so. This did not cause
problems in the former version because
implCreateItemEntry was designed to set a missing/
different WhichID.
I corrected that in SfxFrameItem, also removed not
needed costructor that caused that. Also added a
SAL_WARN and a correction in implCreateItemEntry.
I could have added an assert (did so for running
local UnitTests), but should be enough.
NOTE: When hunting for Items except SfxVoidItem
that get crerated using a WhichID '0' i learned
that this indeed happens: There are some (5) calls
to SfxRequest::SetReturnValue that incarnate an
SfxBoolItem with WhichID '0' (ZERO). This is not
good and I think about how to change that...
Change-Id: I9854a14cdc42d1cc19c7b9df65ce74147d680825
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162124
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
...in include files. This is a mix of automatic rewriting in include files and
manual fixups (mostly addressing loplugin:redundantfcast) in source files that
include those.
Change-Id: I1f3cc1e67b9cabd2e9d61a4d9e9a01e587ea35cc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158337
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
This reverts commit 7fc6063914.
sberg noticed that there is a difference now:
there's a subtle difference now, in that if y was null originally, it would have thrown a (caught) exception, whereas now it will crash in the y.query<X>() call.
Change-Id: Idbb5a08d635d15b5ca63f4822eddf05fb0a5afa0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156002
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
since querying with exceptions is consideably more expensive
Change-Id: I968a9a40766b2abb0d3058549b0ed44011fd5716
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155791
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
to attempt to make it obvious in code what kind of coordinate
system we are dealing with.
The idea is that by doing this, the compile-time type checking
will flush out inconsistencies between different code.
I started with vcl::Window::OutputToAbsoluteScreenPixel
and worked outwards from there.
Change-Id: Ia967d7a0bb38886695f3a761b85c8b9340ddb1c0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154676
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
sometimes it returns a relative position, sometimes an absolute
position.
Rather have two different methods with names that match what
they return.
Change-Id: Ie1e73c6be1c797fd59934c96866d1fef1f972b35
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154653
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Rather than using it's superclass XWindowPeer and implicitly relying on it being XVclWindowPeer and casting it everywhere.
Change-Id: Icfb46f3b920d00f4a167a31803a71bbb0368d05c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149894
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Originally, CentimetersToPoints was implemented as a base method for
Writer and Calc. They are both using the same implementation,
which is correct for Writer, but wrong for Calc.
Since their behavior should be different,
I remove the base method and implement a correct one for Calc.
Change-Id: If957accdd9be86a96ca6fb711502e49a79bf3223
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/148745
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
no need to allocate ShapeHelper separately, it is only one pointer big
Change-Id: Ie4981ca81ac1dd430f22ba32357fcabbbd47bd09
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147944
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Enum to store if document is Writer or Calc because font colors are treated
differently depending on this:
In writer the colors come in as RGB but in calc they are in BGR and both palettes
are in RBG so we only need this conversion for calc.
Includes testdoc for calc - to show this still works
Change-Id: I79d9d585dbfc527c0781543ce1f1095c4db190b7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144788
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
various null checks can be seen to be redundant and removed
Change-Id: Icf49c1de4b0302795d2769a370af3abceaad0221
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147147
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
(See the upcoming commit introducing that loplugin:unocast on why such
dynamic_casts from UNO types are dangerous.)
Change-Id: I913565ccd93472f2a8893c4251e80d0309827c34
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144633
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
(See the upcoming commit introducing that loplugin:unocast on why such
dynamic_casts from UNO types are dangerous.)
Change-Id: I4a66df07f2e768ee3203b730b50be4b9ec6a01a0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144634
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
The unit test just consists of adding a
"WordBasic.MsgBox()" to the vba code in testVBA.docm.
make CppunitTest_sw_macros_test
or just open the file manually for an easy-to-run experience.
Change-Id: I38edfee42649fcc85f0f535a2c9861c45038fa0e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141347
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
A review by Stephan Bergmann made me re-think adding a separate
event for this. It really is only one event and not two
(or three as I initially imagined). In the end, I like this better
because it highlights the difference between Excel and Word
by keeping all the differentiating logic in one place.
The inability to properly document the purpose of these new events
was the impetus to redesign this. Thanks Stephan for the prompt.
Change-Id: Ic2d461c13c4a52e279224cb485d2b6c4a3c57b54
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141233
Tested-by: Justin Luth <jluth@mail.com>
Reviewed-by: Justin Luth <jluth@mail.com>
Unlike Word, ThisWorksheet cannot hold auto-running
subroutines for Open/Close/New.
This fixes a LO 7.4 regression caused by
commit beb6c62e99.
Change-Id: Idb8f72775d9392b306cb924ee776821272b12f3b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141127
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
Note: this should NOT apply to Document_Open which normally
is a private subroutine and runs fine as private.
I tested and it DOES apply to all three version of AutoOpen:
-ThisDocument.AutoOpen,
-<AnyModule>.AutoOpen,
-AutoOpen.Main
Note: this is different from Excel.
Private Auto_Open runs just fine there.
Change-Id: If10c8c90c35275c2b14dc2e15fb357674fc580b8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141114
Tested-by: Justin Luth <jluth@mail.com>
Reviewed-by: Justin Luth <jluth@mail.com>
This is the lowest priority way of running AutoOpen,
and it only applies to Word, not Excel.
There is some code in basic/source/classes/sb.cxx
that was somewhat finding this before it got lost in LO 7.4
with commit beb6c62e99.
TODO: these AutoOpens should only run with a public sub,
not a private one.
Change-Id: I8a733cde13f96636942d84a0b05520692aac3e52
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141094
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
The presence of an Auto_Close module prevents any
auto_close subroutines from running.
Interestingly, Word is different. It doesn't care at all
if such a module is present. (In fact, it uses that
module's main() as an AutoClose if there is no
Sub AutoClose.)
Change-Id: I83a80b7f016dcf2ad3b7fd931acacb6f788241a1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141036
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
...so that its TOOLS_WARN_EXCEPTION can be used in
comphelper/source/misc/logging.cxx in a follow-up commit. (And while at it,
rename from diangose_ex.h to the more appropriate diagnose_ex.hxx. The
comphelper module is sufficiently low-level for this immediate use case, so use
that at least for now; o3tl might be even more suitable but doesn't have a
Library until now. Also, for the immediate use case it would have sufficed to
only break DbgGetCaughtException, exceptionToString, TOOLS_WARN_EXCEPTION,
TOOLS_WARN_EXCEPTION_IF, and TOOLS_INFO_EXCEPTION out of
include/tools/diagnose_ex.h into an additional new
include/comphelper/diagnose_ex.hxx, but its probably easier overall to just move
the complete include file as is.)
Change-Id: I9f3222d4ccf1a9ac29d7eb9ba1530d53e2affaee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138451
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Note that the old conversion was incorrect; it likely followed
MS documentation [1], which is inaccurate: running in Word
MsgBox CentimetersToPoints(1.0)
gives 28.34646 (expected given the 1 in = 72 pt definition).
[1] https://docs.microsoft.com/en-us/office/vba/api/word.application.centimeterstopoints
Change-Id: I4eefccc4bd17958f944f364bc3ea579a8fdbd703
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137855
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
By default Rectangle uses closed interval, if we really want to use half
open intervals then we should specifically say as such in the name.
Change-Id: Id7a91120ba1a1a4bc330014216b73a692dbf03a2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/136575
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>
...where a signed and an unsigned value are compared, and the signed value has
just been proven to be non-negative here
Change-Id: I20600d61a5d59d739bc1bee838c0038e4611aec2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134875
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
for which we have o3tl:: equivalents
Change-Id: I4670fd8b703ac47214be213f41e88d1c6ede7032
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132913
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
look for call sequences that can use string_view and the new o3tl
functions in o3tl/string_view.hxx
Also add a few more wrappers to said #include file
Change-Id: I05d8752cc67a7b55b0b57e8eed803bd06bfcd9ea
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132840
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
so we can use it in places where we cannot include comphelper
Change-Id: Iba0ba3e4c0dcf0f9d1f09092a77c3b2010ec4f6d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132732
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
... to avoid hidden cost of multiple COW checks, because they
call getArray() internally.
This obsoletes [loplugin:sequenceloop].
Also rename toNonConstRange to asNonConstRange, to reflect that
the result is a view of the sequence, not an independent object.
TODO: also drop non-const operator[], but introduce operator[]
in SequenceRange.
Change-Id: Idd5fd7a3400fe65274d2a6343025e2ef8911635d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123518
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
The scenarios are:
1. Calling sequence's begin() and end() in pairs to pass to algorithms
(both calls use getArray(), which does the COW checks)
2. In addition to #1, calling end() again when checking result of find
algorithms, and/or begin() to calculate result's distance
3. Using non-const sequences in range-based for loops, which internally
do #1
4. Assigning sequence to another sequence variable, and then modifying
one of them
In many cases, the sequences could be made const, or treated as const
for the purposes of the algorithms (using std::as_const, std::cbegin,
and std::cend). Where algorithm modifies the sequence, it was changed
to only call getArray() once. For that, css::uno::toNonConstRange was
introduced, which returns a struct (sublclass of std::pair) with two
iterators [begin, end], that are calculated using one call to begin()
and one call to getLength().
To handle #4, css::uno::Sequence::swap was introduced, that swaps the
internal pointer to uno_Sequence. So when a local Sequence variable
should be assigned to another variable, and the latter will be modified
further, it's now possible to use swap instead, so the two sequences
are kept independent.
The modified places were found by temporarily removing non-const end().
Change-Id: I8fe2787f200eecb70744e8b77fbdf7a49653f628
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123542
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Empirically, when we are passing a collection type to a constructor,
80% of the time, we are passing a local temporary that can be moved
instead of being copied.
Change-Id: I5acc9d761c920691934a4be806a3d3ab6cdbab96
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123056
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
enforce it by making the constructor parameter non-default.
Change-Id: I321543e4dcf15ea0a43ad8cce91d2f8dc22df6ec
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122766
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
