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

Speech Dictionary - UI - Regex: Early detect grouping errors (#11407) #11409

Merged
merged 3 commits into from
Sep 18, 2020

Conversation

JulienCochuyt
Copy link
Collaborator

@JulienCochuyt JulienCochuyt commented Jul 23, 2020

Link to issue number:

Fixes #11407

Summary of the issue:

When editing Speech Dictionary entries, regex compilation errors are reported on the fly.
Some grouping errors arise only when actually attempting substitution, leading to the deletion of the faulty entry without prompting the user for a correction.

Description of how this pull request fixes the issue:

Upon editing a regex entry, not only compile it, but also attempt substitution.

Testing performed:

Ensured the error test case in #11407 is properly detected upon submitting the entry.

Known issues with pull request:

Change log entry:

Section: Bug fixes

More regular expression errors are now detected when editing Speech Dictionary entries.

@feerrenrut
Copy link
Contributor

Could you please enable "Allow edits from maintainers" on this PR? It allows us to update the change log and merge all in one.

@JulienCochuyt
Copy link
Collaborator Author

@feerrenrut wrote:

Could you please enable "Allow edits from maintainers" on this PR? It allows us to update the change log and merge all in one.

I'm afraid this seems to work only with forks owned by individuals. See isaacs/github#1681.
Thus, I sent you an invite for write access to accessolutions/nvda.

@feerrenrut
Copy link
Contributor

I'm afraid this seems to work only with forks owned by individuals.

Ah thanks for letting me know, I didn't realize that!

feerrenrut
feerrenrut previously approved these changes Sep 18, 2020
Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks @JulienCochuyt

@feerrenrut feerrenrut merged commit e587fdc into nvaccess:master Sep 18, 2020
@nvaccessAuto nvaccessAuto added this to the 2020.4 milestone Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants