Created attachment 64630 [details] Libreoffice Attached visual representation of problem. When you have same cell in formula more then once, LO gives that cell as many colors as there are mentions in formula when it shoud give it same color for every reference inside formula. Notice that cell E6 has 2 colors in LO, RED and GREEN, when in Excel (attachment no.2) it's always GREEN.
Created attachment 64631 [details] Excel
Created attachment 87688 [details] another example I'm with you! Especially when reviewing huge formulas (I'm an ElectricalEngineering Student) this really turns out as a mess, but even in small scales it is disappointing at least.
Created attachment 87689 [details] sine & root *real case*
Joel and Jan, sorry for bugging you, but I added you as CC of this bug because I saw you were QA in some other bugs I followed. Would this bug be good candidate for Easy Hacks? Not really sure how much work is involved in fixing this bug, but hopefully one should "only" implement code which should check if cell is already present in formula and assign it same color as before. I know it's not first priority bug since it's design bug, but maybe in easy hacks it will get more visiblity.
So unfortunately it is not that easy; but still could be a nice advanced easy hack :-) The color is set in two places: sc/source/ui/app/inputhdl.cxx search for aSet.Put( SvxColorItem( Color( ScRangeFindList::GetColorName( nCount ) ), This sets the color of the text (=B1+B3). sc/source/ui/view/gridwin4.cxx search for Color( ScRangeFindList::GetColorName( i ) ), This sets the color of the higlighted cells. In both cases, you can see that the ScRangeFindList::GetColorName() is being used; it is implemented here: sc/source/ui/app/rfindlst.cxx as ColorData ScRangeFindList::GetColorName( size_t nIndex ) { return aColNames[nIndex % SC_RANGECOLORS]; } You want to improve the implementation so that it does not blindly use the color based on the index, but actually it reuses the already used colors - so you want to make GetColorName non-static, traverse through the entries in the maEntries, and either use the same color (if the entry repeats), or a new one (if it is there for the 1st time). Then you have to update the callers of GetColorName because it is not static any more.
(In reply to comment #5) > So unfortunately it is not that easy; but still could be a nice advanced > easy hack :-) Thank you! Hopefully it will increase bug visibility.
Let me take over the mentoring for this one from Kendy. The solution is actually quite simple. Use a map in ScRangeFindList for maEntries like std::map<ScRange, ScRangeFindData> and storing the color in ScRangeFindData. This also removes the need for the static function ScRangeFindList::GetColorName. Now your insert method does two things(actually std::map::insert will do the work for you): If the range is already in the map return the old color otherwise insert and return the new color. Bonus points for cleaning up gridwin4.cxx:794 and following. Follow-up easy hack if interested could be to make ScInputHandler::InitRangeFinder use one of the existing formula parsers and go through the formula tokens instead of implementing our n-th formula parser.
Rachit Gupta committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=5ff8e1d8e31f23492ee1ccc3af0b73791cd5101b fdo#52461 Multiple instances of same cell now get same color. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
From the above commit, I'll assume this is fixed now and can be closed
Rachit, thank you for enhancement! Please, don't forget to add your self to https://wiki.documentfoundation.org/ReleaseNotes/4.3 Tryed to install dev version from today (windows) but it has some kind of error and it's not possible to run it. Will try again tomorrow.
Tested and it works. Thanks again!
Mikeyy, my pleasure :)
Indeed - but I'm going to open a new request that perhaps Rachit would be interested in. Currently the colors only show when you leave the cell and go back to it, not live. I'll explain on a new bug report and cc Rachit :)
Please see: https://bugs.documentfoundation.org/show_bug.cgi?id=92426 Thank you
Please see for related: https://bugs.documentfoundation.org/show_bug.cgi?id=92441 Thank you
Hello everyone. As much as I would love to work on the issue, I would not be able to do so because now I am busy with my corporate job. If anyone wishes to take over this issue, please go ahead. I will still look into it if and when I get the time. I apologize for the inconvenience. Regards, Rachit
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyInteresting SkillCpp TopicUI ) [NinjaEdit]