[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.documentfoundation.org/show_bug.cgi?id=42897 Laszlo Kis-Adam dfighter1...@gmail.com changed: What|Removed |Added Assignee|libreoffice-b...@lists.free |dfighter1...@gmail.com |desktop.org | -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #15 from Markus Mohrhard markus.mohrh...@googlemail.com --- Markus - is it best to use a heuristic, or have a 'Value' vs. 'Formula/Reference' type radio-button somewhere abouts ? =) The problem with heuristics is that they are often buggy and are forgotten during the next refactoring. I still would prefer to get to the point where we remove the deprecated column/row labels and until then just provide a warning message. Another advantage of using the formula compiler is that it knows all corner cases, and you can then operate just on tokens. -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #12 from Renato Ferreira renato.w...@gmail.com --- (In reply to Markus Mohrhard from comment #11) (In reply to Renato Ferreira from comment #9) Thanks Joel. I am aware of that, but since there seemed to be some uncertainty about the desired behaviour here, I believed it would be better to first discuss it here. And the patch can be used to see how it currently works. If this is the design decision to be made, of course I can go ahead and just push to gerrit. I'd like us to use ScSimpleFormulaCalculator instead of trying to implement a heuristic for formula token detection. That provides access to the ScTokenArray which allows to check how the formula was parsed. Hm, maybe that instead of the isQuoted thing? I did instantiate a compiler elsewhere when a condition is entered to check for double or string. So I think I see, we could also invoke the interpreter to check whether the stored expression evaluates to a string before unquoting it? It does seem more reliable, although double-checking that the quotes are there indeed before removing them also seems to be a good idea, if I understood this correctly. Also, ScSimpleFormulaCalculator does seem very clean to use. Otherwise just changing the background is not enough. We would also need a label with a warning message. We are also no longer use the lcl prefix for local functions in calc, instead please put them into an anonymous namespace. Please note that I did include a label with a warning message in the attached idea. Which also brings another issue to my mind: even if we go ahead and add the equal to value implementation, what happens to begins with, contains, etc.? They still require quotes in order to work properly (do they *only* work with strings?), and this fact would be even more obscured by the added functionality. A few ideas: - Use the warning in all the cases - Add the equal to value condition for convenience, use warning for the others - Auto-correct the begin with, contains etc. to strings if they are not quoted Not sure what would be better... And as Michael already mentioned we should not change ScConditionMode for a hack that is local to the UI code. -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #13 from Renato Ferreira renato.w...@gmail.com --- Looking more closely at it begins with and others also take cell references (so general formulas) as input... So it's basically the same issue as the equal to case, I guess. -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897
--- Comment #14 from Michael Meeks michael.me...@collabora.com ---
Looking more closely at it begins with and others also take cell
references (so general formulas) as input... So it's basically the
same issue as the equal to case, I guess.
Hmm - nasty; incidentally - wrt. your re-factor (which is a big improvement) I
was expecting a table like this:
struct {
ScConditionMode eMode;
int nNumEditFields;
...
} aEntries[] = {
};
that we could use to do both lookups =) really - performance is irrelevant here
and a good representation is more important I think.
Markus - is it best to use a heuristic, or have a 'Value' vs.
'Formula/Reference' type radio-button somewhere abouts ? =)
Anyhow - great to see your code go in Renato.
--
You are receiving this mail because:
You are the assignee for the bug.
___
Libreoffice-bugs mailing list
Libreoffice-bugs@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #11 from Markus Mohrhard markus.mohrh...@googlemail.com --- (In reply to Renato Ferreira from comment #9) Thanks Joel. I am aware of that, but since there seemed to be some uncertainty about the desired behaviour here, I believed it would be better to first discuss it here. And the patch can be used to see how it currently works. If this is the design decision to be made, of course I can go ahead and just push to gerrit. I'd like us to use ScSimpleFormulaCalculator instead of trying to implement a heuristic for formula token detection. That provides access to the ScTokenArray which allows to check how the formula was parsed. Otherwise just changing the background is not enough. We would also need a label with a warning message. We are also no longer use the lcl prefix for local functions in calc, instead please put them into an anonymous namespace. And as Michael already mentioned we should not change ScConditionMode for a hack that is local to the UI code. -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #7 from Renato Ferreira renato.w...@gmail.com --- Created attachment 110355 -- https://bugs.freedesktop.org/attachment.cgi?id=110355action=edit Patch for equal to value vs. equal to formula I have prepared a patch that creates this second behaviour. Basically it adds a temporary type of condition SC_COND_EQUAL_VALUE, which the user can select as equal to value as opposed to formula. Then anything other than numbers or strings is transformed into a string, and the rest of the code uses the regular SC_COND_EQUAL as before. I think this way there is minimal impact on the rest of the code (didn't have to touch the compiler, etc.) I also have a patch for the warning idea above, but I'm not sure myself which idea is better for usability. -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #8 from Joel Madero jmadero@gmail.com --- It needs to be submitted via gerrit - no one will commit it for you from bugzilla. https://wiki.documentfoundation.org/Development/gerrit -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #9 from Renato Ferreira renato.w...@gmail.com --- Thanks Joel. I am aware of that, but since there seemed to be some uncertainty about the desired behaviour here, I believed it would be better to first discuss it here. And the patch can be used to see how it currently works. If this is the design decision to be made, of course I can go ahead and just push to gerrit. -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #10 from Michael Meeks michael.me...@collabora.com --- Renato - wow - this is a great patch =) as Joel says gerrit is best. Then again - that code boggles the mind - those huge switch statements seem unbeleivably horrible =) I'd love to kill those by using a prettier lookup table and a small loop to map to/fro - could you submit that first as a pure re-factor (ie. without the new feature). The other thing is - that I don't think we want to touch the global calc conditional formatting type enumeration with a pseudo-type that is isolated to the dialog; so - I think we should append that to the enumeration some way perhaps by having a 'LAST' value in the enum, and using / adding to that in our table inside the dialog code. But - this is exciting progress =) nice work ! looking forward to your gerrit submits, please CC / poke me there ... -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #5 from Michael Meeks michael.me...@collabora.com --- The problem is that we allow formulas in this place and a string without is a column/range label which might break old systems if we just change it. Interesting =) I wonder if we can add some entries to the combo box more simply: equal to value equal to formula that would make the entry simpler, the difference more discoverable and reduce user-error ? =) Of course - we'd still keep the same underlying enum and do a bit of processing / parsing to select the right version in the dialog ? =) -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #6 from Renato Ferreira renato.w...@gmail.com --- Created attachment 109934 -- https://bugs.freedesktop.org/attachment.cgi?id=109934action=edit Warning idea Here is a possible implementation of the suggestion of adding a warning when the user enters unquoted text. I used yellow in the background following the behaviour that syntax errors make it red. One advantage of this method over splitting the categories is that more ambiguous cases, such as 'TRUE' or 'true', would still work - and the warning disappears if you type that - without having to select between value or formula. However, I also see that this idea, due to not requiring an additional warning, might be the cleaner way. What do you think? I'd be interested in trying to implement whatever is decided, it has been fun so far. What do you think? -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #4 from Markus Mohrhard markus.mohrh...@googlemail.com --- So I propose to just add a warning. The problem is that we allow formulas in this place and a string without is a column/range label which might break old systems if we just change it. We are working on deprecating and possibly removing column and row labels but this will take quite some time. Until then I propose to just issue a warning that this results in something the user might not expect. This has the additional benefit that we don't mess around with the formula compiler which is a huge pain in the long run. Code pointers: moggi [18:40:55] Renato_Ferreira: so there is a ScSimpleFormulaCalculator (in sc/inc/simpleformulacalc.hxx) that you can use to see the result of a formula moggi [18:42:00] Renato_Ferreira: now the idea is to add an handler to the edit that calculates the input in sc/source/ui/condformat/condformatdlgentry.cxx whenever the content of one of the edit boxes changes moggi Renato_Ferreira: I would start with a simple label below the edit box moggi Renato_Ferreira: there is the Edit::SetModifyHdl for registering a handler that listens to changes in the edit moggi Renato_Ferreira: maybe check the range name dialog that does something similar moggi Renato_Ferreira: sc/source/ui/namedlg/namedefdlg.cxx -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 Joel Madero jmadero@gmail.com changed: What|Removed |Added Status|REOPENED|NEW CC||jmadero@gmail.com -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 Michael Meeks michael.me...@collabora.com changed: What|Removed |Added Status|RESOLVED|REOPENED CC||michael.me...@collabora.com See Also||https://bugs.freedesktop.or ||g/show_bug.cgi?id=42282 Resolution|WORKSFORME |--- Ever confirmed|0 |1 Whiteboard|BSA |EasyHack DifficultyBeginner ||SkillCpp TopicCleanup --- Comment #3 from Michael Meeks michael.me...@collabora.com --- This is an annoying one =) Neil just fell over this at Collabora; and I couldn't see what was wrong. AFAICS if someone enters 'foo' in the entry instead of 'foo' - we should warn them and/or (better) auto-correct their input to 'foo' instead of 'foo' =) I imagine that there is some over-complicated thing going on here - perhaps some ability to use defined names or abstract formulae or ... [ no idea ]. But for simple string cases we should definitely improve; fixing the help as per bug#42282 is only a partial solution. sc/source/ui/condformat/condformatdlg.cxx has the code for the conditional formatting dialog. Thanks ! -- You are receiving this mail because: You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 starmat...@yahoo.de changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution||WORKSFORME CC||starmat...@yahoo.de --- Comment #2 from starmat...@yahoo.de 2012-01-19 14:20:58 PST --- Yes, it works like Maciej Rumianowski wrote. I had test it in LibO3.5beta3 You have to write success and with quotation marks as value! Hope i could help you. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
[Libreoffice-bugs] [Bug 42897] FORMATTING: Conditional formatting displays wrong cell background.
https://bugs.freedesktop.org/show_bug.cgi?id=42897 --- Comment #1 from Maciej Rumianowski maciej.rumianow...@gmail.com 2011-12-19 16:14:10 PST --- I have run your test and in step 3 Set Cell value is equal to = success 1. I have typed exacly success with sign and your test fails (conditional formating works perfect) 2. I have typed just success (LibreOffice changes it to 'success') and your test passes (conditional formating is bad) So I think it is connected how text value is treated, here changed to 'text' which is badly evaluated. I think it is also connected to this Bug 42282 -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ Libreoffice-bugs mailing list Libreoffice-bugs@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
