Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate math formula on text change #287

Closed

Conversation

bryanparadis
Copy link
Contributor

@bryanparadis bryanparadis commented Feb 23, 2025

I've been writing a lot of formulae and I am new to the software and not sure about syntax. I've been clicking out of the input box to validate. Looks like I could also be pressing enter. It would be nice if this field was validated as I typed instead and so I have opened this draft pull request.

Validation will run on each change with no delay/debounce. Fiddled around with it a bit and didn't seem super heavy. Don't think there are additional side effects but I am not familiar with the codebase.

@bryanparadis
Copy link
Contributor Author

There are two options for this:

On user and programmatic change

void QLineEdit::textChanged(const QString &text)
This signal is emitted whenever the text changes. The text argument is the new text.
Unlike textEdited(), this signal is also emitted when the text is changed programmatically, for example, by calling setText().
Note: Notifier signal for property text.

On user change only

void QLineEdit::textEdited(const QString &text)
This signal is emitted whenever the text is edited. The text argument is the new text.
Unlike textChanged(), this signal is not emitted when the text is changed programmatically, for example, by calling setText().

@bryanparadis
Copy link
Contributor Author

I've learned to look at the OK button to see if validation is good or not. Maybe something in the field or beside the field instead would be a better/more coupled indicator?

@jankae
Copy link
Owner

jankae commented Feb 24, 2025

Thanks, I think some early validation (and possibly also indicating what the problem is if it fails) is a good idea. Most likely I will checkout this PR locally, add some error indicator and then merge this (unless you want to take this on as well)

@bryanparadis
Copy link
Contributor Author

Thanks, I think some early validation (and possibly also indicating what the problem is if it fails) is a good idea. Most likely I will checkout this PR locally, add some error indicator and then merge this (unless you want to take this on as well)

I added a red background on validation fail to the box and a green on OK. Doesn't look too great. Maybe lighter red and green or some kind of X or checkmark. I can add what I've tried to the branch

@jankae
Copy link
Owner

jankae commented Feb 24, 2025

I think I found a good way to show the status by adding a label under the formula field. Here is what that looks like with a valid formula:
image
And here are various errors with invalid formulas:

  • Empty formula:
    image
  • Using a variable name that does not exist:
    image
  • Using a function that does not exist:
    image
  • Multiple operators in a row:
    image
  • Missing parenthesis:
    image

The error message comes straight out of the muParserX library, so it is not something I can influence. But I think most errors should at least help a little bit when trying to figure out the problem.

@bryanparadis
Copy link
Contributor Author

bryanparadis commented Feb 24, 2025

I think I found a good way to show the status by adding a label under the formula field. Here is what that looks...

Oh interesting!

The error message comes straight out of the muParserX library, so it is not something I can influence. But I think most errors should at least help a little bit when trying to figure out the problem.

Yeah. That is true. I hadn't looked into displaying the parser errors I went for background colors. I broke the formula saving though.

Hahaha. Nuked a line I didn't want to

ui->lMathFormula->setText(t.getMathFormula());

@jankae
Copy link
Owner

jankae commented Feb 24, 2025

I think I'll just merge the original PR + my changes to show the error message. The colors are a nice touch, but I think the error message is more helpful.

I broke the formula saving though.

You just deleted the line which initially fills the QLineEdit with the formula from the trace when opening the dialog.

@jankae jankae closed this Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants