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

[WIP] Implementation and tests for library function for workflow report Markdown label replacement. #17264

Closed
wants to merge 3 commits into from

Conversation

jmchilton
Copy link
Member

@jmchilton jmchilton commented Jan 10, 2024

Extracts a commit of interest out of #17225 and add a commit that does a pretty good job replacing labels in Galaxy Markdown - a bunch of tests to ensure we handle whitespace, quotes, etc... correctly.

The implementation here made it clear to me that #16864 is actually three separate issues. We need to handle input labels changing (labels on input dataset, collection, and parameter steps), workflow output labels (the label in the side form for a node output), and step labels (labels on non-input steps). This might be just two instead of three events in the workflow editor but the editor needs to make that distinction between input and step labels. The output labels are pretty clearly different in implementation.

How to test the changes?

(Select all options that apply)

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@jmchilton jmchilton added kind/bug kind/enhancement kind/refactoring cleanup or refactoring of existing code, no functional changes area/workflows labels Jan 10, 2024
@jmchilton jmchilton force-pushed the markdown_label_replace branch from 379ae1a to 00cb4f2 Compare January 11, 2024 15:27
@jmchilton jmchilton changed the title [WIP] Stub implementation and tests for workflow report Markdown label replacement. [WIP] Implementation and tests for library function for workflow report Markdown label replacement. Jan 11, 2024
@assuntad23
Copy link
Contributor

@jmchilton while Implementing this in #17314, I noticed that the returned markdown removes the double quotes surrounding the label. Is that intentional? Or are they necessary for the report to generate correctly?

const input =
"some random\n`markdown content`\n```galaxy\nhistory_dataset_embedded(footer='cow', output=\"from\", title=dog)\n```\n";
const output =
"some random\n`markdown content`\n```galaxy\nhistory_dataset_embedded(footer='cow', output=to, title=dog)\n```\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

I see in this test case (and others) that you're expecting the double quotes to be dropped. So, I'm assuming that the quotes dissappearance is intentional. It is a little jarring, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just tried this, and broke it, so I think we need to keep the double quotes as delimiters for defining the label.
initial label: input > added it to MD report
changed label to: inputtty > note change in MD report, without double quotes
changed label to inputty bimputty > note: with a space, the MD report, only has inputty with a space, but without the second word bimputty

Copy link
Member Author

Choose a reason for hiding this comment

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

It isn't expected to work that way - it was just a byproduct of the way it does work. I'll see if I can fix it for spaces in a rebase of your branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/workflows kind/bug kind/enhancement kind/refactoring cleanup or refactoring of existing code, no functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants