about magic command

[potoo0]

I has some questions/ideas about kernel, I can contribute to this if we agree on something.

• why not distinguish line magic and cell magic(maybe call them block magic here) like ipython does?
• the order of command exec is not as defined in the cell

  %%
  os.Create("zz.log")
  
  !ls -ahl *.log
  

• hard to contribute magic command
• provide a query command, like lslmagic/lslinemagic lscmagic/lscellmagic
• do not clean main.go if no go magic command

[potoo0]

demo:

package magic

import (
	"fmt"
	"regexp"
	"strings"
	"testing"
)

/*magics:

line:
- !: exec cmd

- %args: cmd arg for main
- %autoget and %noautoget: Default is %autoget, auto get pkg
- %cd [<directory>]: Change current directory of the Go kernel, default it notebook dir
- %env VAR value: available both for Go code and for shell scripts.
- %goflags: go build, "" will reset
- %with_inputs
- %with_password

- %widgets
- %widgets_hb

- %goworkfix:

- %track: [file_or_directory], 		list tracked if no arg
- %untrack [file_or_directory][...] ... => *, list tracked if no arg

- %list/ls
- %remove/rm
- %reset [go.mod]: If the optional go.mod parameter is given, it will re-initialize only the go.mod file

cell:
- %%: 	wrap main func, allow cmd arg, overwrite %args
- %main wrap main func, allow cmd arg, overwrite %args

- %test: current whole cell!!!!!!!!!!!!!!!!!!!!!

- %%bash: Run cells with bash in a subprocess. same with %%script bash
- %%script:
*/

type Magic interface {
	// todo args: kernel.Message, *goexec.State
	Exec() error
}

// todo args: kernel.Message, *goexec.State
type (
	LineExecutor func(*LineMagicContext) error
	CellExecutor func(*CellMagicContext) error
)

type LineMagicContext struct {
	Name        string
	RawArgs     string
	Descriptors string // `*` for project dir
	executor    LineExecutor
}

type CellMagicContext struct {
	Name     string
	RawArgs  string
	Body     []string
	executor CellExecutor
}

func (m *LineMagicContext) Exec() error {
	return m.executor(m)
}

func (m *CellMagicContext) Exec() error {
	return m.executor(m)
}

// magic container
var (
	LineMagics map[string]LineExecutor
	CellMagics map[string]CellExecutor
)

// magic command regexp do not match leading spaces to prevent misjudgment
var (
	lineMagicRegexp = regexp.MustCompile(`^(?P<descriptors>!\*?|%)(?P<name>\w+)\s*(?P<args>.*\S)?\s*`)
	cellMagicRegexp = regexp.MustCompile(`^%%(?P<name>\w*)\s*(?P<args>.*\S)?\s*`)
)

// resolveLineMagic from line, returns lineContinue, magicContext, err
func resolveLineMagic(line string) (bool, *LineMagicContext, error) {
	if matches := lineMagicRegexp.FindStringSubmatch(line); matches != nil {
		name := matches[lineMagicRegexp.SubexpIndex("name")]
		if executor, ok := LineMagics[name]; ok {
			args := matches[lineMagicRegexp.SubexpIndex("args")]
			lineContinue := args != "" && args[len(args)-1] == '\\' && !strings.HasSuffix(args, `\\`)
			magic := &LineMagicContext{
				Name:        name,
				RawArgs:     args,
				Descriptors: matches[lineMagicRegexp.SubexpIndex("descriptors")],
				executor:    executor,
			}
			return lineContinue, magic, nil
		} else {
			return false, nil, fmt.Errorf("line magic %v not found", name)
		}
	}
	return false, nil, nil
}

// resolveCellMagic from line, returns (magicContext, err)
func resolveCellMagic(line string) (*CellMagicContext, error) {
	if matches := cellMagicRegexp.FindStringSubmatch(line); matches != nil {
		name := matches[cellMagicRegexp.SubexpIndex("name")]
		if executor, ok := CellMagics[name]; ok {
			magic := &CellMagicContext{
				Name:     name,
				RawArgs:  matches[cellMagicRegexp.SubexpIndex("args")],
				Body:     make([]string, 0, 8),
				executor: executor,
			}
			return magic, nil
		} else {
			return nil, fmt.Errorf("cell magic %v not found", name)
		}
	}
	return nil, nil
}

func Resolve(src string) []Magic {
	magics := make([]Magic, 0, 4)
	var previous Magic
	lineMagicContinue := false
	for _, line := range strings.Split(src, "\n") {
		// join line if line magic is end with `\`(but not end with `\\`)
		if lineMagic, ok := previous.(*LineMagicContext); ok && lineMagicContinue {
			lineMagic.RawArgs += " " + line
			line = strings.TrimRight(line, " ")
			lineMagicContinue = line != "" && line[len(line)-1] == '\\' && !strings.HasSuffix(line, `\\`)
			continue
		}

		var magic Magic
		// resolve line magic
		lineContinue, lineMagic, err := resolveLineMagic(line)
		if err != nil {
			panic(err)
		}
		lineMagicContinue = lineContinue
		if lineMagic != nil {
			magic = lineMagic
		} else {
			// resolve cell magic
			cellMagic, err := resolveCellMagic(line)
			if err != nil {
				panic(err)
			}
			if cellMagic != nil {
				magic = cellMagic
			}
		}

		if magic != nil {
			magics = append(magics, magic)
			previous = magic
		} else {
			// not magic, append to previous cell magic
			blankLine := strings.TrimLeft(line, " ") == ""
			// create main.go if previous is nil
			if previous == nil && !blankLine {
				previous = &CellMagicContext{
					Name:     "/main.go",
					RawArgs:  "",
					Body:     make([]string, 0, 8),
					executor: _goexec,
				}
				magics = append(magics, previous)
			}
			if cellMagic, ok := previous.(*CellMagicContext); ok &&
				// skip leading blank lines, but force append if no line
				(!blankLine || len(cellMagic.Body) == 0 || strings.TrimLeft(cellMagic.Body[len(cellMagic.Body)-1], " ") != "") {
				cellMagic.Body = append(cellMagic.Body, line)
			}
		}
	}
	return magics
}

/* ------------ tmp debug start ------------ */

func _goexec(m *CellMagicContext) error {
	fmt.Println("goExec", m.RawArgs, len(m.Body), m.Body)
	return nil
}

func _goMainWrapper(m *CellMagicContext) error {
	fmt.Println("goMainWrapper", m.RawArgs, len(m.Body), m.Body)
	return nil
}

/* ------------ tmp debug end   ------------ */

/* ------------ register start ------------ */
func init() {
	RegisterLineMagic([]string{"ls", "list"}, func(m *LineMagicContext) error {
		fmt.Printf("run line magic `%v%v` `%v`\n", m.Descriptors, m.Name, m.RawArgs)
		return nil
	})
	RegisterLineMagic([]string{"pwd"}, func(m *LineMagicContext) error {
		fmt.Printf("run line magic `%v%v` `%v`\n", m.Descriptors, m.Name, m.RawArgs)
		return nil
	})
	RegisterCellMagic([]string{"", "main"}, _goMainWrapper)
}

var (
	MagicIllegal = fmt.Errorf("magic failed to register")
)

func RegisterLineMagic(names []string, executor LineExecutor) error {
	if len(names) < 1 || executor == nil {
		return fmt.Errorf("line %w: names and executor cannot be zero value", MagicIllegal)
	}

	// validate
	hasName := false
	exist := false
	for _, name := range names {
		if !hasName && name != "" {
			hasName = true
		}
		if _, exist = LineMagics[name]; exist {
			break
		}
	}
	if exist {
		return fmt.Errorf("line %w: already exists", MagicIllegal)
	}
	if !hasName {
		return fmt.Errorf("line %w: name cannot be empty", MagicIllegal)
	}

	// init map
	if LineMagics == nil {
		LineMagics = make(map[string]LineExecutor, 12)
	}
	// register
	for _, name := range names {
		if name != "" {
			LineMagics[name] = executor
		}
	}
	return nil
}

func RegisterCellMagic(names []string, executor CellExecutor) error {
	if len(names) < 1 || executor == nil {
		return fmt.Errorf("cell %w: names and executor cannot be zero value", MagicIllegal)
	}

	// validate
	exist := false
	for _, name := range names {
		if _, exist = CellMagics[name]; exist {
			break
		}
	}
	if exist {
		return fmt.Errorf("line %w: already exists", MagicIllegal)
	}

	// init map
	if CellMagics == nil {
		CellMagics = make(map[string]CellExecutor, 12)
	}
	// register
	for _, name := range names {
		CellMagics[name] = executor
	}
	return nil
}

/* ------------ register end   ------------ */

func TestTransform(t *testing.T) {
	src := `


os.Create("zz-1.log")
%%


os.Create("zz-2.log")

!ls -ahl *.log # sas\      
!pwd -k \\
!*pwd
`
	magics := Resolve(src)
	// todo merge goexec/goMainWrapper
	for _, magic := range magics {
		magic.Exec()
	}
}

[potoo0]

@janpfeifer

[janpfeifer]

Sorry, I just saw this, I was merging your changes, with mine -- I did it in parallel ...

So I only learned about "cell magic commands" when you opened the bug, I didn't know about them earlier.

My implementation, just merged, does handle them separately, and checks that they are only usable in the first line of the cell -- seemed a reasonable restriction, but if there is a case where it's not pls let me know.

The new version of the code that handles it is in the script branch -- almost done, but likely I'll only be able to finish it tomorrow.

Testing

I favor functional testing of these: while slower to run, they allow for implementations to change while the test remain the same. See the Integration Tests in nbtests. Hopefully, one can figure out how it works from the examples there.

Registration
That would be nice! I hesitate because rarely the implemetnation is independent. At least with line magic, all commands require creating new the state information and implement else where, so it's not easy to decouple/separate.

The "cell magic" is easier ... but then, there are only two so far: %%writefile and %%script (and the %%bash and %%sh) variants.

Wdyt ?

[potoo0]

Origin cell magic must be at the top of the cell, but current %%(or %main) will wrapping block code into a main func isn't worse.

Considering compatibility and consistency, the idea is as follows:

• line magic: must startswith % or !, only take current line, but allow to use \ to continue(not only work for !, but also %)
• cell magic: must startswith %%cmd(allow cmd empty for main func wrapper), match as many lines as possible until the next magic func.
• replacement:
  • %main => %% or %%main
  • %test => %%test(move to TOP)

About the code decoupling, here is a simple example code for magic command registration/resolution and execution is shown above.

[janpfeifer]

So ... to simplify, what about:

• Cell magic: must start with %% and must be the first line of the cell. It consumes the whole cell.
• Line magic: mostly these are global/cell configuration settings (with exception of !).

And %main and %test remain line magic -- they configure how to run the current cell. And %% is the exception that is not a cell magic.

?

The registration code is simple to do, the question is more whether if it is worth it ? -- so far we only have 2 cell magics, and the line magic won't gain that much.

[potoo0]

Ok, I got it.

It all started because I wanted to implement %%writefile xxx.go for go run xxx.go, but I couldn't read specialcmd.go clearly at first time, so want to generalize all magic function. It may seem a bit unnecessarily over-optimized.

[janpfeifer]

Ugh ... I'm a bit embarrassed of the code ... most grew organically, and was not really planned up-front. But let me know if you have any questions -- we can also schedule a video-chat if needed.

At least it's relatively well documented.

[potoo0]

I have a couple other confusing questions I'd like to discuss with you, could we discuss one by one?

Is this allow? How about keeping execution order?

%%
os.Create("zz.log")

!ls -ahl *.log

[janpfeifer]

This is allowed. The ! and % commands are executed before the go code is parsed.

So the !ls will execute first, then the rest of the cell content follows the standard pass for Go code, which is:

1. Parse it as Go code, with Cell/Line numbers attached.
2. Merge new definitions with existing ones
3. Save everything into one large main.go
4. go imports
5. go get
6. Finally, go run or go test

The output of go imports, go run and go test is parsed for main.go:XX and the line numbers are replaced with a text explaining the cell/line numbers kept in the internal mapping.

[potoo0]

I know the current execution flow, just confirm that you don't disable %main mixed with other commands in the future.
Based on this, it would be nice to ensure the order.

[potoo0]

@janpfeifer
Skip goExec.ExecuteCell if there are non-blank lines, in case main.go is cleaned.

example:
cell-1:

%%
fmt.Println(1)

cell-2:

!*ls

cell-3:

!*cat main.go // main.go#main cleaned

[janpfeifer]

Oh, maybe I was misunderstanding. Do you mean that now the func main() definition changed ?

Notice that the main function changes for every cell execution, it's the only function that is always regenerated, instead of memorized from other cells.

In your example, because cell 2 (or [6]) had some empty lines, it tried to execute the Go code, with an empty main() function.

In the last cell when it cat main.go, it picks up the last main.go generated in cell 2, which was an empty (well, only with the automatically included flag.Parse()).

[janpfeifer]

But it's a good point, I should not call ExecuteCell if the Go contents of the cell are only empty lines.

[potoo0]

Wouldn't it be better to not reset main.go if the cell has no go code(such as just shell command)?

[potoo0]

Yeah, I think so too.

[janpfeifer]

Ok, this one is fixed now:

[potoo0]

@janpfeifer

createGoFileFromLines (https://github.com/janpfeifer/gonb/blob/main/internal/goexec/composer.go#L308 ) should remove w.Write("\t"), use go fmt will better.

[janpfeifer]

Oh yes, I know about this one ... and I debated myself about it.

The \t makes the func main() look better ... with the side effect of breaking multi-line strings. There was another side-effect of removing this tabulation, which I don't recall right now ... Let me try removing the w.Write("\t").

I cannot use go fmt because it can split lines, and then I loose the line mapping from the source code to the cell code -- and also I cannot change the contents of the cells, Jupyter won't allow it.

[janpfeifer]

Should be fixed now:

[potoo0]

Appreciate it!