Created attachment 53863 [details]
proposed patch

no risk involved, so perhaps 3.0 ?

Armin, please take action on this accordingly, please, since most of the changes
are within the DrawingEngine part of the code. Applying the patch should be
straight forward. Thx.

AW->kohei: Thanks for the patch, i will add it to one of my next CWSes. OOo 3.0
is no option, only ShowStoppers get integrated. No way to declarate this one in
that category. I know there is no risk, but RE and testing don't. It is cleanup
of code, it not even fixes a problem.

AW->kohei: Just checked SFX_NOTIFY usage with LXR (see http://lxr.go-oo.org/)
and saw that it is used in MUCH more modules that in svx. Since Your patch only
handles svx, i have to decline Your patch. Please use LXR and enter SFX_NOTIFY
at identifier search to see what i am talking about. A removal of SFX_NOTIFY
only in svx makes no sense for me. Usages are in sfx2, chart2, goodies, sd,
sim2, sip, svx, sc, basctl, basic, starmath, automation and svtools. A
consequent removal of SFX_NOTIFY would need to handle all of this modules and
would need an own CWS from my POV.

kohei->aw: Take a look at where it's defined here

svtools/inc/svtools/lstner.hxx

#define SFX_NOTIFY( rBC, rBCT, rHint, rHintT ) \
		Notify( rBC, rHint )

SFX_NOTIFY simply gets expanded to Notify( rBC, rHint ), which is a concrete
method of SfxListener.  All of SFX_NOTIFY uses in svx are just references and
they are simply expanded to Notify at compile time, and my patch simply does
that manually (hence no risk).

In other words, it's safe to remove just parts of its references as long as we
don't remove the macro definition in lstner.hxx in svtools.  My idea is to
remove all of its references first one module at a time, then once they are all
removed, remove the definition from svtools.  But we don't have to do this all
at once.  We can do this in steps.

AW->kohei: I looked where and how it is define. Actually i myself already once
thought about removing it. My concern is that the function prototype in all
headers using it (in all projects) use the macro. The macro will probably not
change, but having in-between states where it could change and the function
prototype would not match anymore the handish expansion would be bad. Doing the
complete removal in one CWS would exclude that risk.
Of course removing in one project after the other would work, but who guarantees
that this will happen when it's distributed to many people and many CWSes?

What speaks against one CWS and doing it completely (and encapsulated) in one
step there? I would clearly prefer that.

kohei->aw: All uses of SFX_NOTIFY will get expanded to Notify no matter what
module it is used.  So your fear is not warranted.

I just check the exported symbols of libsvxli.so (svxmi.dll on windows), and
there is no SFX_NOTIFY symbols exported, but all symbols are exported as Notify.
 This is expected because that's what macro is supposed to do (to be expanded
before it gets compiled).  I've also checked with all .so files but none of them
export SFX_NOTIFY symbols.

>What speaks against one CWS and doing it completely (and encapsulated) in one
step there? I would clearly prefer that.

It's the enormous overhead that a CWS incurs.  For a feature inclusion or a
collection of bugfixes I wouldn't mind that overhead.  But for something as
simple as this that's an overkill.  So I thought it would be much simpler to
just ask the respective module owner to include the change in their respective CWS.

BTW, that SFX_NOTIFY macro has been there for at least one year.  So, it's safe
to assume that every active CWS has that macro.

Ah, never mind, I'll take care of it.  I still have one active CWS, so I'll just
re-use that then.

AW->kohei: I did not question that the replacement itself is safe (please read
again), i mentioned that it is not safe when someone changes the macro (what
will probably not happen, but is possible). I just gave my comments (two
arguments) why i think doing it in one CWS would be better, but also said Your
method will work (which includes the offer to do it in svx).

One argument for CWSes per se is that something like this can be done isolated
and be added in one single transaction. I also opt against saying it's overkill.
That one CWS would not even get incompatible...

I hate to say it - and You might take me as a nitpicker for this 8-;, but
re-using another CWS for this is also not the theory behind CWSes. Don't take me
wrong: This is theoretically about CWSes, for practical reasons i agree doing
this in less dagerous cases like here. Just my 2 cents...

kohei->aw: I think sending an email to dev@ooo asking all developers to "please
don't change that macro" would've done the job.  But I see your point.  And
there is no argument in the theoretical benefit of CWS.  The biggest thing for
me about CWS is building two install sets and uploading them for QA's retrieval,
mostly because I work from home and my upload speed is miserable.

Anyway, I'll take care of all this in one step.  That means all the other
patches I have posted to other people.

AW->kohei: Then i suggest the following: You create a CWS and checkin all
changes. I can then check it out, build it and provide the install sets locally.
It will need little testing (since it's safe)...?

kohei->aw: Thank you.  I will gladly create a new cws and commit all the
changes.  And yes, it should require little testing since it's a safe change.

fixed in sfxnotifyremoval cws.

kohei->aw: as promised, I've checked in all the changes to sfxnotifyremoval cws,
and that cws is now ready to be checked out on your end.  BTW, who should I
assign as the QA rep for this CWS?

AW->kohei: Whom do You normally assign it to? BTW: I am currently trying to
create the CWS here which is more complicated than i thought. Of course, there
is no tool for it ...

kohei->aw: I usually ask Oliver Craemer if it's Calc related.  I ask because
this cws is not really Calc specific.  (Maybe I can ask him for suggestion...)

And yeah, checking out a CWS can be tricky.  I usually do it manually, but it
would certainly be nice to have a ready-made tool for that (as you said).

There is a nice tool from Kay Ramme to check out a CWS. It needs a small
adjustment ATM. So let me know if you are interested (and let's follow up by
mail in that case).

BTW: thanks for doing the change, I doubt that the macro will be changed in the
next 25 years or so. ;-)

reassign for QA.

verified in internal build cws_sfxnotifyremoval

closed because fix available in build DEV300_m35