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

text/template, html/template: ParseGlob does not document the pattern syntax #30608

Open
dmitshur opened this Issue Mar 5, 2019 · 4 comments

Comments

Projects
None yet
3 participants
@dmitshur
Copy link
Member

dmitshur commented Mar 5, 2019

Packages text/template and html/template have a function named ParseGlob. They have the same documentation:

// ParseGlob creates a new Template and parses the template definitions from the
// files identified by the pattern, which must match at least one file. The
// returned template will have the (base) name and (parsed) contents of the
// first file matched by the pattern. ParseGlob is equivalent to calling
// ParseFiles with the list of files matched by the pattern.
//
// When parsing multiple files with the same name in different directories,
// the last one mentioned will be the one that results.
func ParseGlob(pattern string) (*Template, error)

The documentation of this function does not specify the rules of what the pattern should be. As a result, users reading the documentation cannot know how to use the function.

Both these packages also have a Template.ParseGlob method, which does specify the rules for the pattern:

// ...
// The pattern is processed by filepath.Glob and must match at least one file.
// ...

That information should be copied to the ParseGlob function documentation as well.

/cc @julieqiu @cnoellekb

@empijei

This comment has been minimized.

Copy link
Contributor

empijei commented Mar 5, 2019

This calls filepath.Glob which, in turn, uses filepath.Match syntax.

If we want to document it and make it part of the API I can work on a CL to add it.

/cc @mvdan for the text/template part.

@mvdan

This comment has been minimized.

Copy link
Member

mvdan commented Mar 5, 2019

I don't think we can change the behavior now, so we might as well document it. I'd just make a short mention that this follows the semantics of filepath.Glob to find files.

Perhaps we can change both packages at once, too; I imagine the added godoc sentence would be practically the same.

@dmitshur

This comment has been minimized.

Copy link
Member Author

dmitshur commented Mar 5, 2019

What do you think about starting off by making the ParseGlob function documentation mirror Template.ParseGlob method? I.e.:

 // ParseGlob creates a new Template and parses the template definitions from the
-// files identified by the pattern, which must match at least one file. The
+// files identified by the pattern. The pattern is processed by filepath.Glob
+// and must match at least one file. The
 // returned template will have the (base) name and (parsed) contents of the
 // first file matched by the pattern. ParseGlob is equivalent to calling
 // ParseFiles with the list of files matched by the pattern.
 //
 // When parsing multiple files with the same name in different directories,
 // the last one mentioned will be the one that results.
 func ParseGlob(pattern string) (*Template, error)

Perhaps replacing "pattern is processed by filepath.Glob" with "the syntax of the pattern is the same as in filepath.Match" to shortcut the hops needed.

I think in the short term it's better to continue to refer to filepath.Match syntax rather than copy it here, that way people know it's the same syntax, and it can't get out of sync with filepath package.

@empijei

This comment has been minimized.

Copy link
Contributor

empijei commented Mar 5, 2019

Sounds good to me. If you want to work on it feel free to send a CL with that change on both packages, for all 4 functions. I believe it is better to do this in one change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.