Hi Well, I also looked in the archives ( because I only had the last 2 messages in my mbox ) and I noticed Niklas's comment about using ScRange::ParseAny in ScRangeList::Parse - this *seems* to do exactly the right thing ( of course additionally the address = address + ':' address has to be removed also ) and is a much better solution than my 'hacky' patch. Is is possible to make this change? are there any risks? I ran all my set of vba addressing test and these seem to work fine with the change above ( including the new cases that initiated this thread )
if necessary we could to mitigate risks by just using the ScRange::ParseAny for the XL_XX addressing variants opinions? Noel On Mon, 2007-10-22 at 12:35 +0100, Noel Power wrote: > Hi Niklas/Eike > > On Mon, 2007-07-02 at 12:00 +0100, Noel Power wrote: > > Hi, > > > > Finally go a chance to apply the changes from the cws and glad to say > > these changes don't seem to affect the tests that I have :-) > > > > Thanks, > > > > Noel > but then I got a problem that forced me to write consider another > test-case ;-) Unfortunately this breaks things. I will try and explain > ( please forgive my unfortunate mixing up of the terms > range/address/reference etc. ) I am afraid I never really know which to > use when ( /me must ask google sometime ) > > The problem I see is with singe cell range references that contain a > sheet reference that are passed to ScRangeList::Parse e.g. things like > Sheet1!A1 > > as we know ScRangeList::Parse creates ranges from these type of > references by doing address = address + ':' + address > > this results in addresses like 'Sheet1!A1:Sheet1!A1' in the parse_XL_A1 > case. > *normally* such a reference will successfully parse, but this is just > by luck :-( because you could have a reference with a sheet ref where > the sheet name is a valid XL_A1 address, then there is trouble e.g. if > there is a sheet named b4, and 'b4!a3' is passed to ScRangeList::Parse() > then what lcl_ScRange_Parse_XL_A1 actually will get will be 'b4!a3:b4! > a3' and this will actually get parsed as if it is 'a3:b4' :-( clearly > not what we want > > for R1C1 addresses the situation is far worse because an address like > 'Sheet!RC' gets sent to lcl_ScRange_Parse_XL_R1C1 as 'Sheet!RC:Sheet!RC' > and such an address will never parse ( because of slight differences in > the parsing algorithm for R1C1 ), additionally the same problem exists > in that it is possible to construct a sheet reference that is a valid > R1C1 address to create a false range e.g. r8c24!R1C1 > > so initially I tried to get rid of the code in ScRangeList ( that adds ) > the ':' thinking that I could use the bOnlyAcceptSingle ( which is false > for ScRange::Parse ) to trigger treating the singe cell reference as a > range ( where start = end ) but I ran into other problems further down > the stack ( with formula parsing ) that seem to expect the current > behaviour. Also the logic for handling bOnlyAcceptSingle seems slightly > different in lcl_ScRange_Parse_XL_A1 & lcl_ScRange_Parse_XL_R1C1. My > conclusion is that > a) although I think disabling that address -> range conversion in > ScRangeList::Parse is a good idea. But one would need to be able to tell > lcl_ScRange_Parse_XL_XXXX routines to do the right thing with single > cell references that need to be treated as a range ( start = end ) it > seems that they 'nearly' can do that already > b) making changes in this area can have very subtle side affects if you > don't know what you are doing ( I fall into the latter category ) and my > attempt to do a) above failed > c) realising my incompetence I decided to leave a better solution to > those who know what they are doing and offer a temporary solution > instead. The patch should > a) behave as before unless presented with the unexpected sheet ref > b) when it sees the 'unexpected' sheet ref then the parser just skips > it > > the patch is here > http://svn.gnome.org/viewvc/ooo-build/trunk/patches/ooxml/xmlfilter-fixup-singlerange-sheetref.diff > ( or I can upload it somewhere but there is no issue and creating an > issue against something that is incomplete seems a bit daft ) > > Anyway, sorry for the long and rambling barely coherent mail, feel free > to ping/poke me on IRC ( for fast response ) etc. if further details are > required > > Noel > > > > > On Fri, 2007-06-29 at 09:41 +0100, Noel Power wrote: > > > Hi Niklas > > > On Wed, 2007-06-27 at 17:43 +0200, Niklas Nebel wrote: > > > > Noel Power wrote: > > > > > If you can let me know when you commit the change ( and to what > > > > > file(s) ) I will at least run some tests against that as a sanity > > > > > check. > > > > > > > > It's in sc/source/core/tool/address.cxx 1.9.18.1. > > > > I also made a small change to compiler.cxx (1.68.18.1) to correct the > > > > output of such references. > > > Unless I am very smart (which I'm not) it's unlikely that I will finish > > > what I am currently busy with :-( So it will be Monday before I get to > > > try this > > > > > > Noel > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
