Address review feedback

internal/cmd/fmt.go

@@ -87,7 +87,7 @@ func fmtFiles(files []string, flatten bool, formatJSON bool, write bool, outputt
 		var formatted []byte
 
 		if flatten {
-			notebook, err := editor.Deserialize(logger, data, identityResolver)
+			notebook, err := editor.Deserialize(data, editor.Options{LoggerInstance: logger, IdentityResolver: identityResolver})
 			if err != nil {
 				return errors.Wrap(err, "failed to deserialize")
 			}
@@ -101,7 +101,7 @@ func fmtFiles(files []string, flatten bool, formatJSON bool, write bool, outputt
 				}
 				formatted = buf.Bytes()
 			} else {
-				formatted, err = editor.Serialize(logger, notebook, nil)
+				formatted, err = editor.Serialize(notebook, nil, editor.Options{LoggerInstance: logger})
 				if err != nil {
 					return errors.Wrap(err, "failed to serialize")
 				}

internal/cmd/run.go

@@ -188,8 +188,8 @@ func runCmd() *cobra.Command {
 
 			for _, task := range runTasks {
 				doc := task.CodeBlock.Document()
-				fmtr := doc.Frontmatter()
-				if err := doc.FrontmatterError(); err != nil {
+				fmtr, err := doc.FrontmatterWithError()
+				if err != nil {
 					return err
 				}
 				if fmtr != nil && fmtr.SkipPrompts {

internal/cmd/tui.go

@@ -140,10 +140,10 @@ func tuiCmd() *cobra.Command {
 				task := result.task
 
 				doc := task.CodeBlock.Document()
-				if err := doc.FrontmatterError(); err != nil {
+				fmtr, err := doc.FrontmatterWithError()
+				if err != nil {
 					return err
 				}
-				fmtr := doc.Frontmatter()
 
 				if fmtr != nil && !fmtr.SkipPrompts {
 					err = promptEnvVars(cmd, runnerClient, task)

internal/command/config_code_block.go

@@ -46,8 +46,7 @@ func (b *configBuilder) dir() string {
 	var dirs []string
 
 	doc := b.block.Document()
-	err := doc.FrontmatterError()
-	fmtr := doc.Frontmatter()
+	fmtr, err := doc.FrontmatterWithError()
 	if err == nil && fmtr != nil && fmtr.Cwd != "" {
 		dirs = append(dirs, fmtr.Cwd)
 	}
@@ -70,8 +69,7 @@ func (b *configBuilder) programPath() (programPath string) {
 	// If the language is a shell language, check frontmatter for shell overwrite.
 	if isShellLanguage(language) {
 		doc := b.block.Document()
-		fmtr := doc.Frontmatter()
-		err := doc.FrontmatterError()
+		fmtr, err := doc.FrontmatterWithError()
 		if err == nil && fmtr != nil && fmtr.Shell != "" {
 			programPath = fmtr.Shell
 		}

internal/config/autoconfig/autoconfig.go

@@ -187,11 +187,10 @@ func getProjectFilters(c *config.Config) ([]project.Filter, error) {
 		case config.FilterTypeDocument:
 			filters = append(filters, project.Filter(func(t project.Task) (bool, error) {
 				doc := t.CodeBlock.Document()
-				err := doc.FrontmatterError()
+				fmtr, err := doc.FrontmatterWithError()
 				if err != nil {
 					return false, err
 				}
-				fmtr := doc.Frontmatter()
 				if fmtr == nil {
 					return false, nil
 				}

internal/project/project.go

@@ -491,10 +491,9 @@ func getCodeBlocksFromFile(path string) (document.CodeBlocks, error) {
 func getCodeBlocks(data []byte) (document.CodeBlocks, error) {
 	identityResolver := identity.NewResolver(identity.DefaultLifecycleIdentity)
 	d := document.New(data, identityResolver)
-	fm := d.Frontmatter()
-	err := d.FrontmatterError()
+	fmtr, err := d.FrontmatterWithError()
 
-	if err == nil && fm != nil && !fm.Runme.IsEmpty() && fm.Runme.Session.GetID() != "" {
+	if err == nil && fmtr != nil && !fmtr.Runme.IsEmpty() && fmtr.Runme.Session.GetID() != "" {
 		return nil, nil
 	}
 

internal/runner/client/client_local.go

@@ -89,8 +89,7 @@ func (r *LocalRunner) setupSession() error {
 func (r *LocalRunner) newExecutable(task project.Task) (runner.Executable, error) {
 	block := task.CodeBlock
 	doc := task.CodeBlock.Document()
-	fmtr := doc.Frontmatter()
-	err := doc.FrontmatterError()
+	fmtr, err := doc.FrontmatterWithError()
 	if err != nil {
 		return nil, err
 	}

internal/runner/client/client_remote.go

@@ -182,8 +182,7 @@ func (r *RemoteRunner) ResolveProgram(ctx context.Context, mode runnerv1.Resolve
 func (r *RemoteRunner) RunTask(ctx context.Context, task project.Task) error {
 	block := task.CodeBlock
 	doc := task.CodeBlock.Document()
-	fmtr := doc.Frontmatter()
-	err := doc.FrontmatterError()
+	fmtr, err := doc.FrontmatterWithError()
 	if err != nil {
 		return err
 	}

pkg/document/document.go

@@ -116,7 +116,7 @@ func (d *Document) splitAndParse() error {
 	return nil
 }
 
-func (d *Document) FrontMatterRaw() []byte {
+func (d *Document) FrontmatterRaw() []byte {
 	return d.frontmatterRaw
 }
 

pkg/document/document_test.go

@@ -86,8 +86,7 @@ First paragraph
 		assert.Equal(t, []byte("First paragraph"), doc.Content())
 		assert.Equal(t, 20, doc.ContentOffset())
 
-		frontmatter := doc.Frontmatter()
-		err = doc.FrontmatterError()
+		frontmatter, err := doc.FrontmatterWithError()
 		require.NoError(t, err)
 		marshaledFrontmatter, err := frontmatter.Marshal(testIdentityResolver.DocumentEnabled())
 		require.NoError(t, err)
@@ -142,8 +141,7 @@ shell = "fish"
 				_, err := doc.Root()
 				require.NoError(t, err)
 
-				frontmatter := doc.Frontmatter()
-				err = doc.FrontmatterError()
+				frontmatter, err := doc.FrontmatterWithError()
 				assert.NoError(t, err)
 				assert.Equal(t, "fish", frontmatter.Shell)
 			})
@@ -164,8 +162,7 @@ shell = "fish"
 		err := doc.Parse()
 		require.NoError(t, err)
 
-		frontmatter := doc.Frontmatter()
-		err = doc.FrontmatterError()
+		frontmatter, err := doc.FrontmatterWithError()
 		assert.ErrorContains(t, err, "failed to parse frontmatter content: yaml: line 3: found unexpected end of stream")
 		assert.Nil(t, frontmatter)
 	})

pkg/document/editor/editor.go

@@ -18,19 +18,30 @@ const (
 	DocumentID     = "id"
 )
 
-func Deserialize(log *zap.Logger, data []byte, identityResolver *identity.IdentityResolver) (*Notebook, error) {
+type Options struct {
+	IdentityResolver *identity.IdentityResolver
+	LoggerInstance   *zap.Logger
+}
+
+func (o Options) Logger() *zap.Logger {
+	if o.LoggerInstance == nil {
+		o.LoggerInstance = zap.NewNop()
+	}
+	return o.LoggerInstance
+}
+
+func Deserialize(data []byte, opts Options) (*Notebook, error) {
 	// Deserialize content to cells.
-	doc := document.New(data, identityResolver)
+	doc := document.New(data, opts.IdentityResolver)
 	node, err := doc.Root()
 	if err != nil {
 		return nil, err
 	}
 
-	frontmatter := doc.Frontmatter()
-	fmErr := doc.FrontmatterError()
+	frontmatter, fmErr := doc.FrontmatterWithError()
 	// non-fatal error
 	if fmErr != nil {
-		log.Warn("failed to parse frontmatter", zap.Error(fmErr))
+		opts.Logger().Warn("failed to parse frontmatter", zap.Error(fmErr))
 	}
 
 	notebook := &Notebook{
@@ -43,23 +54,23 @@ func Deserialize(log *zap.Logger, data []byte, identityResolver *identity.Identi
 
 	// Additionally, put raw frontmatter in notebook's metadata, no matter invalid or valid
 	// TODO(adamb): handle the error.
-	if raw, err := frontmatter.Marshal(identityResolver.DocumentEnabled()); err == nil && len(raw) > 0 {
+	if raw, err := frontmatter.Marshal(opts.IdentityResolver.DocumentEnabled()); err == nil && len(raw) > 0 {
 		notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, FrontmatterKey)] = string(raw)
 	}
 	// if parsing frontmatter failed put unparsed frontmatter in notebook's metadata to avoid earsing it with "default frontmatter"
-	if raw := doc.FrontMatterRaw(); fmErr != nil && len(raw) > 0 {
+	if raw := doc.FrontmatterRaw(); fmErr != nil && len(raw) > 0 {
 		notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, FrontmatterKey)] = string(raw)
 	}
 
 	// Store internal ephemeral document ID if the document lifecycle ID is disabled.
-	if !identityResolver.DocumentEnabled() {
-		notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, DocumentID)] = identityResolver.EphemeralDocumentID()
+	if !opts.IdentityResolver.DocumentEnabled() {
+		notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, DocumentID)] = opts.IdentityResolver.EphemeralDocumentID()
 	}
 
 	return notebook, nil
 }
 
-func Serialize(log *zap.Logger, notebook *Notebook, outputMetadata *document.RunmeMetadata) ([]byte, error) {
+func Serialize(notebook *Notebook, outputMetadata *document.RunmeMetadata, opts Options) ([]byte, error) {
 	var result []byte
 	var err error
 	var frontmatter *document.Frontmatter
@@ -72,7 +83,7 @@ func Serialize(log *zap.Logger, notebook *Notebook, outputMetadata *document.Run
 		frontmatter, err = document.ParseFrontmatter(raw)
 		// non-fatal error
 		if err != nil {
-			log.Warn("failed to parse frontmatter", zap.Error(err))
+			opts.Logger().Warn("failed to parse frontmatter", zap.Error(err))
 		}
 	}