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

New function: file.Walk #281

Merged
merged 11 commits into from
Apr 13, 2018
Merged

Conversation

christopher-avila
Copy link

No description provided.

@hairyhenderson
Copy link
Owner

Hi @christopher-avila, thank you for this contribution!

Can you please describe what you're aiming for with file.FindFiles? You should be able to accomplish the same effect with a template similar to this:

$ gomplate -i '{{ range file.ReadDir "." }}{{ if not (file.IsDir .) }}{{ . }} IS A FILE{{"\n"}}{{ end }}{{ end -}}'
Gopkg.toml IS A FILE
LICENSE IS A FILE
CHANGELOG.md IS A FILE
...

As for strings.TrimPrefix - that seems like a reasonable addition, but I'd rather it be adapted to work better in a pipeline. This means that I should be able to use it in a template like {{ print "Hello, World!" | strings.TrimPrefix "Hello, " }}. To do this the signature needs its arguments swapped so the input string is last. See the strings.Trim function - https://github.com/hairyhenderson/gomplate/pull/281/files#diff-3011984e758821c934ac913b336f80d3L82 - as an example.

Ideally I like to also have tests and documentation added for any new functions. If you're not comfortable with these I can add them in after.

Thanks!

@christopher-avila
Copy link
Author

Hi @hairyhenderson ! Thanks for your answer!

file.FindFiles is different from file.ReadDir, if I'm not wrong, because file.ReadDir is not recursive, and read everything inside a given directory. file.FindFiles is more focused on give the user the name and path of all the files inside the given directory.

For example:

$ mkdir -p /tmp/example
$ mkdir -p /tmp/example/a
$ mkdir -p /tmp/example/b
$ mkdir -p /tmp/example/c
$ touch /tmp/example/file_1
$ touch /tmp/example/a/file_2
$ touch /tmp/example/c/file_3

$ bin/gomplate -i '{{ range file.FindFiles "/tmp/example" }}{{ . }}{{"\n"}}{{ end -}}'
/tmp/example/a/file_2
/tmp/example/c/file_3
/tmp/example/file_1

On the other hand, we have file.ReadDir:

$ bin/gomplate -i '{{ range file.ReadDir "/tmp/example" }}{{ . }}{{"\n"}}{{ end -}}'
a
b
file_1
c

In addition, file.FindFiles is quiet on undefined or non-existing directories, giving you an empty array of names in case the file or directory doesn't exists. Following the previous example:

$ mkdir -p /tmp/example
$ mkdir -p /tmp/example/a
$ mkdir -p /tmp/example/b
$ mkdir -p /tmp/example/c
$ touch /tmp/example/file_1
$ touch /tmp/example/a/file_2
$ touch /tmp/example/c/file_3

$ bin/gomplate -i '{{ range file.FindFiles "/tmp/example/d" }}{{ . }}{{"\n"}}{{ end -}}'
$ ....

On the other hand, we have file.ReadDir:

$ bin/gomplate -i '{{ range file.ReadDir "/tmp/example/d" }}{{ . }}{{"\n"}}{{ end -}}'
Error: template: <arg>:1:13: executing "<arg>" at <file.ReadDir>: error calling ReadDir: open /tmp/example/d: no such file or directory
Usage:
  gomplate [flags]

Flags:
  -d, --datasource datasource      datasource in alias=URL form. Specify multiple times to add multiple sources.
  -H, --datasource-header header   HTTP header field in 'alias=Name: value' form to be provided on HTTP-based data sources. Multiples can be set.
      --exclude stringArray        glob of files to not parse
  -f, --file file                  Template file to process. Omit to use standard input, or use --in or --input-dir (default [-])
  -h, --help                       help for gomplate
  -i, --in string                  Template string to process (alternative to --file and --input-dir)
      --input-dir directory        directory which is examined recursively for templates (alternative to --file and --in)
      --left-delim delimiter       override the default left-delimiter [$GOMPLATE_LEFT_DELIM] (default "{{")
  -o, --out file                   output file name. Omit to use standard output. (default [-])
      --output-dir directory       directory to store the processed templates. Only used for --input-dir (default ".")
      --right-delim delimiter      override the default right-delimiter [$GOMPLATE_RIGHT_DELIM] (default "}}")
  -v, --version                    print the version

For the case of trimPrefix I can do what you proposed, if you agree the changes :)

And for the tests, I can do them, or I can leave them for you, as you prefer :)

Really thanks for your time.

@robermorales
Copy link

Any update on this?

funcs/strings.go Outdated
@@ -43,6 +43,7 @@ func AddStringFuncs(f map[string]interface{}) {
f["split"] = strings.Split
f["splitN"] = strings.SplitN
f["trim"] = strings.Trim
f["trimPrefix"] = strings.TrimPrefix
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer not to add new non-namespaced functions nowadays. Please remove this line.

@hairyhenderson
Copy link
Owner

@christopher-avila @robermorales sorry for the delay. Thanks for adding the TrimPrefix tests. That looks good to me. I thought I'd mentioned it before, but I clearly didn't - I'd much rather merge the 2 new functions as separate PRs (to keep history clean).

Can you propose the strings.TrimPrefix function in a new PR? That way I'll get it merged faster.

As for file.FindFiles, I understand the use-case better now. I think it would be more flexible to have a file.Walk function instead, with essentially the same semantics as file.ReadDir, except that it's recursive:

$ gomplate -i '{{ range file.Walk "." }}{{ if not (file.IsDir .) }}{{ . }} IS A FILE{{"\n"}}{{ end }}{{ end -}}'
README.md IS A FILE
aws/ec2info.go IS A FILE
aws/ec2meta.go IS A FILE
aws/testutils.go IS A FILE
base64/base64.go IS A FILE
...

What do you think?

@christopher-avila christopher-avila changed the title New functions proposal: files.FindFiles and trimPrefix New function proposal: files.Walk Apr 12, 2018
@christopher-avila
Copy link
Author

I'm modifying the files.FindFiles function, naming it files.Walk. It looks like a better name for the function (is actually a wrapper over filepath.Walk). I'll commit this changes on this PR and will open another one for strings.TrimPreffix.

Thanks for your time!

@christopher-avila
Copy link
Author

From: https://ci.appveyor.com/project/hairyhenderson/gomplate/build/637

--- FAIL: TestFileWalk (0.00s)
	Error Trace:	file_test.go:47
	Error:      	Not equal: 
	            	expected: []string{"/tmp", "/tmp/bar", "/tmp/bar/baz"}
	            	actual  : []string{"/tmp", "\\tmp\\bar", "\\tmp\\bar\\baz"}

Any idea of this? =/

@hairyhenderson
Copy link
Owner

Thanks @christopher-avila!

I'm thinking the issue has to do with Windows path separators. If you instead use filepath.Join to create the expected paths you should in theory be OK. I'm not sure why the first element is /tmp though... Perhaps an odd quirk in how afero's Walk is implemented.

@christopher-avila
Copy link
Author

I think it is solved. Take a look!

Thanks for your time! :)

@hairyhenderson hairyhenderson merged commit f867f35 into hairyhenderson:master Apr 13, 2018
@hairyhenderson
Copy link
Owner

Thanks again for the contribution, @christopher-avila!

@hairyhenderson hairyhenderson changed the title New function proposal: files.Walk New function proposal: file.Walk Apr 13, 2018
@christopher-avila
Copy link
Author

Thanks to you for your time!

@hairyhenderson hairyhenderson changed the title New function proposal: file.Walk New function: file.Walk May 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants