Skip to content
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

syntax: Provide default.yaml as fallback definition #2933

Merged
merged 10 commits into from
Apr 18, 2024
34 changes: 25 additions & 9 deletions cmd/micro/micro_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ func handleEvent() {
if e != nil {
screen.Events <- e
}
DoEvent()

for len(screen.DrawChan()) > 0 || len(screen.Events) > 0 {
DoEvent()
}
}

func injectKey(key tcell.Key, r rune, mod tcell.ModMask) {
Expand Down Expand Up @@ -151,6 +154,16 @@ func openFile(file string) {
injectKey(tcell.KeyEnter, rune(tcell.KeyEnter), tcell.ModNone)
}

func findBuffer(file string) *buffer.Buffer {
var buf *buffer.Buffer
for _, b := range buffer.OpenBuffers {
if b.Path == file {
buf = b
}
}
return buf
}

func createTestFile(name string, content string) (string, error) {
testf, err := ioutil.TempFile("", name)
if err != nil {
Expand Down Expand Up @@ -190,14 +203,7 @@ func TestSimpleEdit(t *testing.T) {

openFile(file)

var buf *buffer.Buffer
for _, b := range buffer.OpenBuffers {
if b.Path == file {
buf = b
}
}

if buf == nil {
if findBuffer(file) == nil {
t.Errorf("Could not find buffer %s", file)
return
}
Expand Down Expand Up @@ -236,6 +242,11 @@ func TestMouse(t *testing.T) {

openFile(file)

if findBuffer(file) == nil {
t.Errorf("Could not find buffer %s", file)
return
}

// buffer:
// base content
// the selections need to happen at different locations to avoid a double click
Expand Down Expand Up @@ -299,6 +310,11 @@ func TestSearchAndReplace(t *testing.T) {

openFile(file)

if findBuffer(file) == nil {
t.Errorf("Could not find buffer %s", file)
return
}

injectKey(tcell.KeyCtrlE, rune(tcell.KeyCtrlE), tcell.ModCtrl)
injectString(fmt.Sprintf("replaceall %s %s", "foo", "test_string"))
injectKey(tcell.KeyEnter, rune(tcell.KeyEnter), tcell.ModNone)
Expand Down
100 changes: 73 additions & 27 deletions internal/buffer/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,64 @@ func calcHash(b *Buffer, out *[md5.Size]byte) error {
return nil
}

func parseDefFromFile(f config.RuntimeFile, header *highlight.Header) *highlight.Def {
data, err := f.Data()
if err != nil {
screen.TermMessage("Error loading syntax file " + f.Name() + ": " + err.Error())
return nil
}

if header == nil {
header, err = highlight.MakeHeaderYaml(data)
if err != nil {
screen.TermMessage("Error parsing header for syntax file " + f.Name() + ": " + err.Error())
return nil
}
}

file, err := highlight.ParseFile(data)
if err != nil {
screen.TermMessage("Error parsing syntax file " + f.Name() + ": " + err.Error())
return nil
}

syndef, err := highlight.ParseDef(file, header)
if err != nil {
screen.TermMessage("Error parsing syntax file " + f.Name() + ": " + err.Error())
return nil
}

return syndef
}

// findRealRuntimeSyntaxDef finds a specific syntax definition
// in the user's custom syntax files
func findRealRuntimeSyntaxDef(name string, header *highlight.Header) *highlight.Def {
for _, f := range config.ListRealRuntimeFiles(config.RTSyntax) {
if f.Name() == name {
syndef := parseDefFromFile(f, header)
if syndef != nil {
return syndef
}
}
}
return nil
}

// findRuntimeSyntaxDef finds a specific syntax definition
// in the runtime files
func findRuntimeSyntaxDef(name string, header *highlight.Header) *highlight.Def {
for _, f := range config.ListRuntimeFiles(config.RTSyntax) {
if f.Name() == name {
syndef := parseDefFromFile(f, header)
if syndef != nil {
return syndef
}
}
}
return nil
}

// UpdateRules updates the syntax rules and filetype for this buffer
// This is called when the colorscheme changes
func (b *Buffer) UpdateRules() {
Expand Down Expand Up @@ -710,6 +768,10 @@ func (b *Buffer) UpdateRules() {
var header *highlight.Header
// search for the syntax file in the user's custom syntax files
for _, f := range config.ListRealRuntimeFiles(config.RTSyntax) {
if f.Name() == "default" {
continue
}

data, err := f.Data()
if err != nil {
screen.TermMessage("Error loading syntax file " + f.Name() + ": " + err.Error())
Expand Down Expand Up @@ -766,7 +828,7 @@ func (b *Buffer) UpdateRules() {
}

if !foundDef {
// search in the default syntax files
// search for the syntax file in the runtime files
for _, f := range config.ListRuntimeFiles(config.RTSyntaxHeader) {
data, err := f.Data()
if err != nil {
Expand Down Expand Up @@ -844,29 +906,7 @@ func (b *Buffer) UpdateRules() {

if syntaxFile != "" && !foundDef {
// we found a syntax file using a syntax header file
for _, f := range config.ListRuntimeFiles(config.RTSyntax) {
if f.Name() == syntaxFile {
data, err := f.Data()
if err != nil {
screen.TermMessage("Error loading syntax file " + f.Name() + ": " + err.Error())
continue
}

file, err := highlight.ParseFile(data)
if err != nil {
screen.TermMessage("Error parsing syntax file " + f.Name() + ": " + err.Error())
continue
}

syndef, err := highlight.ParseDef(file, header)
if err != nil {
screen.TermMessage("Error parsing syntax file " + f.Name() + ": " + err.Error())
continue
}
b.SyntaxDef = syndef
break
}
}
b.SyntaxDef = findRuntimeSyntaxDef(syntaxFile, header)
}

if b.SyntaxDef != nil && highlight.HasIncludes(b.SyntaxDef) {
Expand All @@ -876,9 +916,10 @@ func (b *Buffer) UpdateRules() {
for _, f := range config.ListRuntimeFiles(config.RTSyntax) {
data, err := f.Data()
if err != nil {
screen.TermMessage("Error parsing syntax file " + f.Name() + ": " + err.Error())
screen.TermMessage("Error loading syntax file " + f.Name() + ": " + err.Error())
continue
}

header, err := highlight.MakeHeaderYaml(data)
if err != nil {
screen.TermMessage("Error parsing syntax file " + f.Name() + ": " + err.Error())
Expand Down Expand Up @@ -907,9 +948,14 @@ func (b *Buffer) UpdateRules() {
if b.Highlighter == nil || syntaxFile != "" {
if b.SyntaxDef != nil {
b.Settings["filetype"] = b.SyntaxDef.FileType
} else {
// search for the default file in the user's custom syntax files
b.SyntaxDef = findRealRuntimeSyntaxDef("default", nil)
if b.SyntaxDef == nil {
// search for the default file in the runtime files
b.SyntaxDef = findRuntimeSyntaxDef("default", nil)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about de-duplicating the code? The size of UpdateRules() is getting scary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started the refactoring in #3206. This PR will benefit of it too in the moment of necessary rebase.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My work on #3208 made me realize that such refactoring is probably is not an easy thing to do, unfortunately. The logic in UpdateRules() is quite subtle, it's not obvious to me how to abstract any important pieces of it into separate functions without loss of functionality and correctness.

Perhaps we should approach this from a different direction: instead of trying to restructure the code, just add some helpers to the syntax parser API, e.g. ParseDefFromFile() (which would internally just call ParseFile() and then ParseDef()), maybe also FindRuntimeFile() and FindRealRuntimeFile() or something like that, to make the code of UpdateRules() at least shorter and a bit easier to read.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...] just add some helpers to the syntax parser API, e.g. ParseDefFromFile() (which would internally just call ParseFile() and then ParseDef()), maybe also FindRuntimeFile() and FindRealRuntimeFile() or something like that, to make the code of UpdateRules() at least shorter and a bit easier to read.

Yep, this will reduce the duplication of load/parses and logs in that way that they aren't spread all over UpdateRules().
I will take care of this in the moment #3208 is merged, because then the whole change needs a rebase and I will drop some lines I did.

}
} else {
b.SyntaxDef = &highlight.EmptyDef
}

if b.SyntaxDef != nil {
Expand Down
3 changes: 0 additions & 3 deletions pkg/highlight/highlighter.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ func combineLineMatch(src, dst LineMatch) LineMatch {
// A State represents the region at the end of a line
type State *region

// EmptyDef is an empty definition.
var EmptyDef = Def{nil, &rules{}}

// LineStates is an interface for a buffer-like object which can also store the states and matches for every line
type LineStates interface {
LineBytes(n int) []byte
Expand Down
10 changes: 10 additions & 0 deletions runtime/syntax/default.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
filetype: unknown

detect:
filename: ""

rules:
# Mails
- special: "[[:alnum:].%_+-]+@[[:alnum:].-]+"
# URLs
- identifier: "(https?|ftp|ssh)://\\S*[^])>\\s,.]"
Loading