[IMP] errors: display error origin position #5413
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
LucasLefevre
force-pushed
the
master-error-origin-lul
branch
from
9f6c114 to
30f3c79
Compare
Purpose ------- Let's say a cell is evaluated to an error because one of its inputs is an error itself. The tooltip shows the error message, but you don't know where it comes from. If the formula has many reference, you have to check every one of those references to find the original error. As an example, if the formula =IF(OR(AND($D21<G$17,$D21>I$1),AND(I$1<$E21,I$1>$D21)),1,"") results with an error. Where does it come from ? D21 ? J1 ? I1 ? E21 ? Spec ---- Add below the error message the cell from which the error is coming from. The cell reference should be clickable to jump to cell. Task: 4452745
LucasLefevre
force-pushed
the
master-error-origin-lul
branch
from
30f3c79 to
078bee3
Compare
rrahir
reviewed
Jan 7, 2025
| title: _t("Error"), | ||
| message: cell.message, | ||
| }); | ||
| evaluationError = cell; | ||
| } | ||
|
|
||
| const validationErrorMessage = getters.getInvalidDataValidationMessage(position); |
There was a problem hiding this comment.
just inject that value directly to the errorTooltip component as you did with the EvaluationError. The builder is no longer relevant if it only manages a single case. Or you could keep the builder and have it yield a component... but the former seems easier
LucasLefevre
commented
Jan 7, 2025
Comment on lines
-1089
to
-1090
| // TO DO: add tests for exp format (ex: 4E10) | ||
| // RO DO: add tests for DATE string format (ex match: "28 02 2020") |
There was a problem hiding this comment.
5 years old TODOs... 😛
LucasLefevre
force-pushed
the
master-error-origin-lul
branch
2 times, most recently
from
ef182a0 to
a7bf79f
Compare
Since the previous commit, the ErrorToolTip component is no longer completely independant from the source of errors (it needs to know the evaluation error). So the remaining use of the "generic part" is now only for data validation. A generic system for a single use case is not a generic system. Let's make it more direct and move everything to the component. Task: 4452745
LucasLefevre
force-pushed
the
master-error-origin-lul
branch
from
a7bf79f to
dba11ab
Compare
|
robodoo rebase-ff |
|
Merge method set to rebase and fast-forward. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Purpose
Let's say a cell is evaluated to an error because one of its inputs is an error itself. The tooltip shows the error message, but you don't know where it comes from.
If the formula has many reference, you have to check every one of those references to find the original error.
As an example, if the formula =IF(OR(AND($D21<G$17,$D21>I$1),AND(I$1<$E21,I$1>$D21)),1,"") results with an error. Where does it come from ? D21 ? J1 ? I1 ? E21 ?
Spec
Add below the error message the cell from which the error is coming from. The cell reference should be clickable to jump to cell.
Task: 4452745
review checklist