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

fix: crash when generating CSV/TSV output #1507

Merged
merged 3 commits into from
Jul 11, 2024
Merged

Conversation

ivan-aksamentov
Copy link
Member

@ivan-aksamentov ivan-aksamentov commented Jul 10, 2024

Follow up of #1492

Prompted by the user report over email, this solves a couple of bugs in the CSV/TSV output code. These are both improper lookups/insertions into data structures.

Additionally, I incremented the version in the local storage entry name in Nextclade Web, such that all settings are reset on user's machines after release. This will make the new columns introduced in #1492 (released in 3.8.0) to be enabled by default (otherwise entries which are missing are treated as "disabled", which should probably be changed).

These bugs highlight the need for some love and refactoring of the CSV output generation code, which is one of the oldest modules in the codebase.

We can ignore unknown categories here. We check correctness of this input elsewhere.
This allows to erase current CSV/TSV column config settings, such that the default set of columns is selected. This is done to enable new column categories released in Nextclade 3.8.0.
Copy link

vercel bot commented Jul 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nextclade ✅ Ready (Inspect) Visit Preview Jul 10, 2024 10:20am

@ivan-aksamentov ivan-aksamentov changed the title fix: panic in map lookup fix: crash when generating CSV/TSV output Jul 10, 2024
@ivan-aksamentov ivan-aksamentov merged commit 454e143 into master Jul 11, 2024
20 checks passed
@ivan-aksamentov ivan-aksamentov deleted the fix/csv-output-crash branch July 11, 2024 02:49
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.

1 participant