using GetDefinitionForFilename incrementally

[mvdan]

Thanks for this library! I'm looking at using it to implement mvdan/sh#393.

Here's one problem, though. When one runs a command like shfmt -l -w ., it searches all shell files under the current directory, similar to what gofmt would do.

Imagine a fairly simple use case, where we have a few hundred shell files and an .editorconfig in the current directory. As I read the source for GetDefinitionForFilename, I'm a bit horrified, as using it for each of the shell script paths would result in the library re-reading and re-parsing the editorconfig hundreds of times.

I think the library should expose a stateful, incremental API. Keeping data in a struct, and using methods, it could cache:

• which directories are known to contain (or not contain) an editorconfig
• editorconfig files that have already been parsed

For my sample use case above, this would mean only parsing the editorconfig once, and also only one stat call per directory containing a shell script.

For the sake of simplicity, merging of editorconfig files doesn't need to be cached.

[mvdan]

If this sounds like a good idea and noone plans to work on it, I might send a patch myself.

[mvdan]

Here's a draft of what the API could look like:

type IncrementalFinder struct {
    ConfigName string

    // unexported fields
    configsByDir map[string]*EditorConfig
}

func (*IncrementalFinder) DefinitionForFilename(filename string) (*EditorConfig, error)

[andreynering]

Hi @mvdan,

This suggestion is totally reasonable.

My counter-proposal on the API would be something like:

type Finder {
	// unexported fields only
}

func New(r io.Reader) (*Finder, error) {
	// ...
}

func (*Finder) Find(filename string) (*EditorConfig, error) {
	// ...
}

[mvdan]

I don't think that API would cut it; it's constrained to a single io.Reader, whereas I want to cache multiple parsed editorconfig files at once.

[andreynering]

Closing this since you're making your own library for this anyway.

I don't think we'll have that soon in this one.