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

Closed
dmitshur opened this issue Mar 5, 2019 · 5 comments
Closed
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

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

@dmitshur dmitshur added Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 5, 2019
@dmitshur dmitshur added this to the Go1.13 milestone Mar 5, 2019
@empijei
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
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
Copy link
Contributor 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
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.

@adg adg self-assigned this May 31, 2019
@gopherbot
Copy link

Change https://golang.org/cl/179739 mentions this issue: html/template, text/template: document glob semantics

@golang golang locked and limited conversation to collaborators Jun 16, 2020
@rsc rsc unassigned adg Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants