-
Notifications
You must be signed in to change notification settings - Fork 47
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
Skip files that include Runme session IDs (those are output files) #459
Conversation
Please review as per description, @adambabik. |
I don't quite understand "[...] saving outputs available ahead of load (deserialization)". Is it to cover the case of a new or modified and unsaved file? |
internal/project/project.go
Outdated
@@ -410,6 +410,11 @@ func getCodeBlocksFromFile(path string) (document.CodeBlocks, error) { | |||
func getCodeBlocks(data []byte) (document.CodeBlocks, error) { | |||
identityResolver := identity.NewResolver(identity.DefaultLifecycleIdentity) | |||
d := document.New(data, identityResolver) | |||
|
|||
if f, _ := d.Frontmatter(); f != nil && f.Runme.Session.ID != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: To be completely safe in the case the API changes, I recommend:
if f, _ := d.Frontmatter(); f != nil && f.Runme.Session.ID != "" { | |
if f, err := d.Frontmatter(); err == nil && f != nil && f.Runme.Session.ID != "" { |
internal/project/project.go
Outdated
@@ -410,6 +410,11 @@ func getCodeBlocksFromFile(path string) (document.CodeBlocks, error) { | |||
func getCodeBlocks(data []byte) (document.CodeBlocks, error) { | |||
identityResolver := identity.NewResolver(identity.DefaultLifecycleIdentity) | |||
d := document.New(data, identityResolver) | |||
|
|||
if f, _ := d.Frontmatter(); f != nil && f.Runme.Session.ID != "" { | |||
return document.CodeBlocks{}, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hint: This is more idiomatic and does not allocate any memory (negligible in this case):
return document.CodeBlocks{}, nil | |
return nil, nil |
Neither. It's to get closer to make stateful/vscode-runme#996 an opt-out versus opt-in feature (which it currently is). Runme knows how to write markdowns with cell outputs but can't do the reverse (yet). See screenshot in PR for clarity. |
The extension will write Outputs to a separate session file. The idea is to make saving outputs available ahead of load (deserialization) and chunk up the dev effort required to unlock both.
Since, by default, the Session Outputs file will be colocated with its original document, it's highly likely to be included when a project is being loaded. The extension UX rejects running Session Outputs files in the notebook UX (stateful/vscode-runme#1025).
This PR introduces a check for a Runme session ID, which will skip these docs/tasks accordingly.
PS: Also brings back a launch cfg to attach to dlv which is required for tui debugging.