Skip to content

Fix Syntax Error with MermaidRenderer #5935

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

Merged
merged 4 commits into from
Apr 14, 2025

Conversation

Lehmann-Fabian
Copy link
Contributor

When I create an HTML of the DAG using the MermaidRenderer, I get a Syntax Error when opening the file.
Quoting the names solves this.

Copy link

netlify bot commented Apr 2, 2025

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit 13ab173
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/67fcc968eee56700081eca4c

@bentsherman
Copy link
Member

Thanks Fabian. Which version did this happen for you? Trying to determine if it was a regression

@Lehmann-Fabian
Copy link
Contributor Author

I think it cannot cause an error with Nextflow as it is because Tasknames and Operators don't contain spaces.
However, I faced the problem when I reused the DAG renders of Nextflow for my nf-datatrail plugin.
I defined the physical tasks as processes, e.g., TASKNAME (1). In this case, you get a Syntax error. I can send you an example via Mail.
Quoting the name fixes it.

Therefore, I suggest to accept my changes to make it:

  1. More stable better and better reusable
  2. Generating a consistent output file. For ORIGINS and NODES, you have already quoted the labels.

Copy link
Member

@bentsherman bentsherman left a comment

Choose a reason for hiding this comment

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

Please allow me to update the branch or update it yourself so that I can merge it.

Keep in mind that every time another PR is merged to master, this PR has to be updated again. Because of our time difference we could get caught in an infinite loop. This is why it's easier to give me push access so that I can update the branch myself

@Lehmann-Fabian
Copy link
Contributor Author

Please go ahead.

@bentsherman bentsherman merged commit e3b8ca4 into nextflow-io:master Apr 14, 2025
21 of 22 checks passed
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