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.

Sign up for GitHub

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
Merged

syntax: Provide default.yaml as fallback definition #2933

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
430da61
highlighter: Remove EmptyDef since it's superseeded by a nil check of…
JoeKar Sep 20, 2023
3907942
syntax: Provide default.yaml as fallback definition
JoeKar Sep 18, 2023
f265179
buffer: Correct error message in case of failed read
JoeKar Sep 21, 2023
2c53d1f
test: Perform DoEvent() as long as normal or draw events are present
JoeKar Sep 22, 2023
6ffabd6
buffer: Let the user override the default.yaml
JoeKar Mar 17, 2024
87ee41a
buffer: Don't process the default syntax in the user's custom file lo…
JoeKar Apr 17, 2024
4cafa60
syntax: Optimize the patterns and remove the comment region
JoeKar Mar 17, 2024
ed993a4
buffer: Precise comment about searching in the internal runtime files
JoeKar Mar 19, 2024
089160a
buffer: Refactor `UpdateRules()` by creating `parseDefFromFile()`
JoeKar Apr 14, 2024
6cd39ef
buffer: Refactor `UpdateRules()` by creating further helper functions
JoeKar Apr 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
buffer: Refactor UpdateRules() by creating further helper functions 
- `findRealRuntimeSyntaxDef()`
- `findRuntimeSyntaxDef()`

This will reduce the length of this function again and thus improves the
readability.
  • Loading branch information
@JoeKar
JoeKar committed Apr 18, 2024
commit 6cd39efddcece5dd0aa7aa993618fbfd71c3b14e
65 changes: 31 additions & 34 deletions internal/buffer/buffer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,34 @@ func parseDefFromFile(f config.RuntimeFile, header *highlight.Header) *highlight
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 @@ -878,17 +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 {
syndef := parseDefFromFile(f, header)
if syndef == nil {
continue
}

b.SyntaxDef = syndef
break
}
}
b.SyntaxDef = findRuntimeSyntaxDef(syntaxFile, header)
}

if b.SyntaxDef != nil && highlight.HasIncludes(b.SyntaxDef) {
Expand Down Expand Up @@ -932,31 +950,10 @@ func (b *Buffer) UpdateRules() {
b.Settings["filetype"] = b.SyntaxDef.FileType
} else {
// search for the default file in the user's custom syntax files
for _, f := range config.ListRealRuntimeFiles(config.RTSyntax) {
if f.Name() == "default" {
syndef := parseDefFromFile(f, nil)
if syndef == nil {
continue
}

b.SyntaxDef = syndef
break
}
}

b.SyntaxDef = findRealRuntimeSyntaxDef("default", nil)
if b.SyntaxDef == nil {
// search for the default file in the runtime files
for _, f := range config.ListRuntimeFiles(config.RTSyntax) {
if f.Name() == "default" {
syndef := parseDefFromFile(f, nil)
if syndef == nil {
continue
}

b.SyntaxDef = syndef
break
}
}
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.

}
}
Expand Down
Loading