Fix 1455 and retain invalid frontmatter over failing/overwriting #627
Conversation
|
Lemme know what you think @adambabik. Making checking for errors optional seemed like the best way forward to solve for the PR's description/linked issue. |
pkg/document/editor/editor.go
Outdated
| @@ -17,17 +18,19 @@ const ( | |||
| DocumentID = "id" | |||
| ) | |||
|
|
|||
| func Deserialize(data []byte, identityResolver *identity.IdentityResolver) (*Notebook, error) { | |||
| func Deserialize(log *zap.Logger, data []byte, identityResolver *identity.IdentityResolver) (*Notebook, error) { | |||
There was a problem hiding this comment.
Suggestion: as this interface will likely get extended, I suggest this:
| func Deserialize(log *zap.Logger, data []byte, identityResolver *identity.IdentityResolver) (*Notebook, error) { | |
| type Options struct { | |
| IdentityResolver *identity.IdentityResolver | |
| Logger *zap.Logger | |
| } | |
| func Deserialize(data []byte, opts Options) (*Notebook, error) { |
pkg/document/document.go
Outdated
| @@ -116,6 +116,10 @@ func (d *Document) splitAndParse() error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (d *Document) FrontMatterRaw() []byte { | |||
There was a problem hiding this comment.
Suggestion: Let's stick with Frontmatter for consistency.
| func (d *Document) FrontMatterRaw() []byte { | |
| func (d *Document) FrontmatterRaw() []byte { |
pkg/document/frontmatter.go
Outdated
| return d.frontmatter | ||
| } | ||
|
|
||
| func (d *Document) FrontmatterError() error { |
There was a problem hiding this comment.
Suggestion: We can have two methods: Frontmatter() *Frontmatter and FrontmatterWithError() (*Frontmatter, error). I think it will be a bit more clear and the caller will only need to use one method when it wants to handle frontmatter's error.
| func (d *Document) FrontmatterError() error { | |
| func (d *Document) FrontmatterWithError() (*Frontmatter, error) { |
pkg/project/formatter.go
Outdated
| @@ -30,7 +33,7 @@ func Format(files []string, basePath string, flatten bool, formatJSON bool, writ | |||
| identityResolver := identity.NewResolver(identity.DefaultLifecycleIdentity) | |||
|
|
|||
| if flatten { | |||
| notebook, err := editor.Deserialize(data, identityResolver) | |||
| notebook, err := editor.Deserialize(logger, data, identityResolver) | |||
There was a problem hiding this comment.
Remark: With my earlier suggestion, this will become:
| notebook, err := editor.Deserialize(logger, data, identityResolver) | |
| notebook, err := editor.Deserialize(data, editor.Options{IdentityResolver: identityResolver}) |
The logger can be figure out by editor.Deserialize and default to noop.
|
@adambabik feedback makes perfect sense 👌. Thank you. |
sourishkrout
force-pushed
the
fix-1455-invalid-frontmatter
branch
from
6a62664 to
45a1f4b
Compare
|
Turns out people use templating languages and interpolation in frontmatter which makes it invalid unless rendered first. The idea of this PR is to pass-thru frontmatter when attempts to parsing it fail.
Fixes stateful/vscode-runme#1455