Open mvdbeek opened 2 years ago
I've been debugging this one for some time and, so far, I found out that the issue seems to be related to the markdown not being properly resolved in resolve_invocation_markdown
For example, workflow_display()
should be resolved to something like workflow_display(workflow_id={id})
but the backend returns workflow_display()
and the client then fails because it assumes the workflow_id
must be present.
My first assumption is that something may have changed that makes the markdown syntax in WORKFLOW_WITH_CUSTOM_REPORT_1 fail to resolve but I'll keep digging.
Ok, so I think I got to the bottom of this... in summary, the backend is resolving the markdown correctly but the client, in splitMarkdown, is not considering a particular case that this WORKFLOW_WITH_CUSTOM_REPORT_1
test workflow is "featuring".
This workflow markdown report template contains examples about how to actually write embedded galaxy markdown blocks like so:
I assume from the text above that the first workflow_display()
block is not meant to be resolved (because of the extra indentation?) and instead be treated as a literal, but the splitMarkdown
function is not considering this "literal" block and then tries to create embedded elements for them that will fail because the backend didn't resolve them (on purpose).
How do we want to proceed here?
Right now there seem to be some discrepancies between the markdown syntax that the backend and frontend can handle around galaxy fenced code blocks.
splitMarkdown
in the client to support these "indented code blocks as literals"? This sounds nice for meta documentation but, right now, I don't see any real usage value for end-users.Thanks for figuring out what was going on there.
I think the additional indentation to delineate literal code blocks seems odd (reminds me of rst actually), I think in other markdown flavors you'd just add another set of backticks, but increase the number by one. e.g in github flavored markdown:
I'm a code block
which I wrote as
I'm a code block
I wonder if that actually works now, then that would probably be the best fix ?
I think in other markdown flavors you'd just add another set of backticks
That's a great solution and I like it much better.
I think the underlying markdown-it
library supports this but I just tried and it still doesn't work probably because splitMarkdown is simply looking for ```galaxy blocks regardless of the surrounding context.
Should we try to rewrite splitMarkdown
to be a bit smarter and deal with that? I'm happy to try but don't know if this can or is worth being done before the freeze.
Nah, definitely not worth doing before the release with other user-facing bugs open, I think that's so far only a testing issue. We could also try to hook into into the markdown-it renderer or token stream (https://github.com/markdown-it/markdown-it/blob/master/docs/development.md), that might be the most robust way to do it.
Describe the bug The markdown component doesn't seem to properly wait for values to become defined, and I believe that causes testing issues in https://github.com/galaxyproject/galaxy/pull/13174, and in general is noisy and suboptimal:
full console log:
``` vue.runtime.esm.js?2b0e:619 [Vue warn]: Invalid prop: type check failed for prop "history_dataset_id". Expected String with value "undefined", got Undefined found in --->Galaxy Version and/or server at which you observed the bug Galaxy Version: (check/api/version if you don't know)
Commit: (run
git rev-parse HEAD
if you run this Galaxy server)To Reproduce Steps to reproduce the behavior:
Expected behavior The markdown component should not start fetching items until the id is defined.
Additional context The actual selenium errors are likely due to the returned list of datatypes being larger, and therefor there are longer response times. Still, this is not a huge list and should easily work.