Skip to content
New issue

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

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

Already on GitHub? # to your account

[errors-] create Error(s)Sheet only if it is not already open #2671

Merged
merged 1 commit into from
Jan 20, 2025

Conversation

midichef
Copy link
Contributor

@midichef midichef commented Jan 8, 2025

Fixes an inconvenience noted by @daviewales as part of #2573:

if I keep pressing Ctrl+E, duplicate error sheets are created, rather than just jumping me back to the original one.

@midichef
Copy link
Contributor Author

Should I use an explicit source=None parameter when creating the sheets, in ErrorsSheet("errors_all") and ErrorSheet("errors_recent")? They're getting it initialized like that from BaseSheet already.

@saulpw
Copy link
Owner

saulpw commented Jan 15, 2025

Should I use an explicit source=None parameter when creating the sheets, in ErrorsSheet("errors_all") and ErrorSheet("errors_recent")? They're getting it initialized like that from BaseSheet already.

No, let them use the default.

@anjakefala
Copy link
Collaborator

I found a bug with this fix!

To reproduce, put 1/0 in your ~/.visidatarc.
Press ^E, and it will correctly open the ZeroDivisionError.

Then type a column without numerical values with #. Use z^E on one of the cells, and it will incorrectly load the ZeroDivisionError as the cell error.

@midichef
Copy link
Contributor Author

Oops, nice catch. I fixed it by letting self.source override the default sources of vd.lastErrors[-1] (in ErrorSheet) and vd.lastErrors (in ErrorsSheet). But note my style question here.

@midichef
Copy link
Contributor Author

midichef commented Jan 19, 2025

Based on feedback by @daviewales I replaced the short-circuiting and with the more common if/else. This PR is complete now.

Copy link
Collaborator

@anjakefala anjakefala left a comment

Choose a reason for hiding this comment

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

Yup, everything works!

@anjakefala anjakefala merged commit 8f8aaec into saulpw:develop Jan 20, 2025
14 checks passed
@midichef midichef deleted the error_recent_all branch January 21, 2025 01:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants