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

Jump to bottom

proposal: path/filepath: add TrimExt #25012

Closed
kybin opened this issue Apr 23, 2018 · 6 comments
Closed

proposal: path/filepath: add TrimExt #25012

kybin opened this issue Apr 23, 2018 · 6 comments
Labels
FrozenDueToAge Proposal
Milestone
Proposal

Comments

@kybin
Copy link
Contributor

kybin commented Apr 23, 2018 •
edited

It would be good to add filepath.TrimExt function.

Which will do as it's name says. For example,

filepath.TrimExt("some/path/main.go") // returns "some/path/main"
filepath.TrimExt("main.go") // returns "main"

Reason

We often trims an extension from a path but there is no API for this, so we do like this now.

path[:len(path)-len(filepath.Ext(path))] 
// or
strings.TrimSuffix(path, filepath.Ext(path))

Which is cumbersome to type and hard to read.

The second one is easier to read, but user should import "strings" package for it.

Implemetation

It's implemation will be quite small.

func TrimExt(path string) string {
    return path[:len(path)-len(Ext(path))]
}

If this proposal accepted, I could work for this.

Note

The "path" package also has Ext. But it is not frequently used, so I don't think it should be also added. If I am wrong, please correct me.
@gopherbot gopherbot added this to the Proposal milestone Apr 23, 2018
@agnivade
Copy link
Contributor

Given that you can easily get this functionality by using existing standard library functions, I don't see much value for this to exist in standard library. Mostly because it fails https://golang.org/doc/faq#x_in_std test.

Also, adding one more line makes it much more readable.

extLen := len(filepath.Ext(path))
path[:len(path)-extLen]

Although, IMO the one-liner is perfectly readable.

@rsc
Copy link
Contributor

rsc commented Apr 23, 2018

path[:len(path)-len(filepath.Ext(path))]

Can you show that this comes up a lot in code today? I can't think of any time I've ever used this expression. Usually I write something like strings.TrimSuffix(path, ".go")+".o", where there's exactly one suffix that can be trimmed but others must not be.

@kybin
Copy link
Contributor Author

kybin commented Apr 24, 2018

Thank you @agnivade for your thought.

I think we are used to it, but there is a room for making it better.

For the x_in_std test, I think it's small, complement addition to the "filepath" package. And it's maintenance cost seems not much.

@kybin
Copy link
Contributor Author

kybin commented Apr 24, 2018

Thanks @rsc . I should've verify that first.

I did quick check with go-corpus data.

I removed vendored results, if they are already included.

With your argument about strings.TrimSuffix(path, ".go")+".o", I excluded the similar cases.

It's not tremendous, still many. But I don't know you think they are enough.

-len(filepath.Ext(path)) pattern

$ rg "[-]len\(filepath.Ext"

github.com/spf13/hugo/tpl/template.go
285:	name = name[:len(name)-len(filepath.Ext(innerPath))] + ".html"

github.com/zyedidia/micro/cmd/micro/rtfiles.go
42:	return fn[:len(fn)-len(filepath.Ext(fn))]

github.com/yosssi/ace/cmd/ace/main.go
42:	base := baseFile[:len(baseFile)-len(filepath.Ext(baseFile))]
46:		inner = innerFile[:len(innerFile)-len(filepath.Ext(innerFile))]

$ rg "[-]len\(ext"

github.com/golang/protobuf/protoc-gen-go/generator/generator.go
320:		name = name[:len(name)-len(ext)]

github.com/pingcap/pd/_vendor/vendor/github.com/unrolled/render/render.go
182:				name := (rel[0 : len(rel)-len(ext)])
227:				name := (rel[0 : len(rel)-len(ext)])

github.com/ulikunitz/xz/cmd/gxz/file.go
137:		target = path[:len(path)-len(ext)]

github.com/CodisLabs/codis/vendor/github.com/martini-contrib/render/render.go
221:				name := (r[0 : len(r)-len(ext)])

github.com/spf13/hugo/hugolib/permalinks.go
159:	//var name = p.Source.Path()[0 : len(p.Source.Path())-len(extension)]

github.com/spf13/hugo/target/file.go
65:	return f[:len(f)-len(ext)]

github.com/gogo/protobuf/protoc-gen-gogo/generator/generator.go
356:		name = name[:len(name)-len(ext)]

github.com/ncw/rclone/b2/api/types.go
76:	base := remote[:len(remote)-len(ext)]
90:	base := remote[:len(remote)-len(ext)]

github.com/ncw/rclone/fs/operations.go
939:	base := remote[:len(remote)-len(ext)]

github.com/couchbase/go-couchbase/tools/loadfile/loadfile.go
52:	return file[0 : len(file)-len(ext)]

github.com/git-lfs/git-lfs/lfsapi/ssh.go
83:			basessh = basessh[:len(basessh)-len(ext)]

github.com/openshift/origin/test/integration/template_test.go
104:			name = name[:len(name)-len(ext)]

github.com/openshift/origin/vendor/gopkg.in/natefinch/lumberjack.v2/lumberjack.go
233:	prefix := filename[:len(filename)-len(ext)]
371:	filename = filename[:len(filename)-len(ext)]
393:	prefix = filename[:len(filename)-len(ext)] + "-"

github.com/openshift/origin/examples/examples_test.go
50:			name = name[:len(name)-len(ext)]

github.com/kataras/go-template/html/html.go
215:			root := name[:len(name)-len(ext)]

github.com/kardianos/service/example/runner/runner.go
115:	name := execname[:len(execname)-len(ext)]

golang.org/x/tools/go/gcimporter15/gcimporter_test.go
118:					name := f.Name()[0 : len(f.Name())-len(ext)] // remove extension

golang.org/x/tools/blog/blog.go
192:		p = p[len(root) : len(p)-len(ext)] // trim root and extension

$ rg "[-]len\(\"[.]"

github.com/klauspost/compress/flate/huffman_bit_writer_test.go
34:			out = in[:len(in)-len(".in")] + ".golden"

github.com/xenolf/lego/providers/dns/gandi/gandi.go
92:	name := fqdn[:len(fqdn)-len("."+authZone)]

github.com/xenolf/lego/providers/dns/exoscale/exoscale.go
129:	name = name[:len(name)-len("."+zone)]

github.com/gopherjs/gopherjs/tests/run.go
856:	filename = filename[:len(filename)-len(".go")]

github.com/chrislusf/seaweedfs/weed/storage/disk_location.go
29:		base := name[:len(name)-len(".dat")]

github.com/iris-contrib/lego/providers/dns/gandi/gandi.go
92:	name := fqdn[:len(fqdn)-len("."+authZone)]

github.com/newrelic/go-agent/internal/sysinfo/docker.go
83:			id = string(cols[2][len("/system.slice/docker-") : len(cols[2])-len(".scope")])

golang.org/x/tools/cmd/stringer/endtoend_test.go
61:		typeName := fmt.Sprintf("%c%s", name[0]+'A'-'a', name[1:len(name)-len(".go")])

golang.org/x/tools/cmd/godoc/codewalk.go
230:			v = append(v, &elem{name[0 : len(name)-len(".xml")], cw.Title})

golang.org/x/image/font/sfnt/sfnt_test.go
269:	} else if want := filename[:len(filename)-len(".ttf")]; name != want {

strings.TrimSuffix(... filepath.Ext(path)) pattern

$ rg "strings.TrimSuffix\(.*[, ]+filepath[.]Ext.*\)"

github.com/spf13/hugo/tpl/template.go
317:		templateName := strings.TrimSuffix(name, filepath.Ext(name)) + ".html"

github.com/onsi/ginkgo/ginkgo/testsuite/test_suite.go
38:	packageName := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path))

github.com/future-architect/vuls/commands/history.go
86:			fileBase := strings.TrimSuffix(f.Name(), filepath.Ext(f.Name()))

github.com/docker/docker/pkg/plugins/discovery.go
37:			name := strings.TrimSuffix(fi.Name(), filepath.Ext(fi.Name()))
50:			name := strings.TrimSuffix(fi.Name(), filepath.Ext(fi.Name()))

github.com/docker/docker/vendor/github.com/docker/notary/trustmanager/keystore.go
263:	name := strings.TrimSpace(strings.TrimSuffix(filepath.Base(keyID), filepath.Ext(keyID)))

github.com/flynn/flynn/test/cluster/instance.go
174:	name := strings.TrimSuffix(filepath.Base(image), filepath.Ext(image))

github.com/flynn/flynn/vendor/github.com/jvatic/asset-matrix-go/scss-asset.go
153:					_p = strings.TrimSuffix(_p, filepath.Ext(_p)) + "-" + a.r.cacheBreaker + filepath.Ext(_p)

$ rg "strings.TrimSuffix\(.*[, ]+ext.*\)"

github.com/influxdata/influxdb/tsdb/engine/tsm1/tombstone.go
324:		filename = strings.TrimSuffix(filename, ext)

github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/genswagger/generator.go
49:		base := strings.TrimSuffix(name, ext)

github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/descriptor/registry.go
296:		return strings.TrimSuffix(base, ext)

github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/gengateway/generator.go
79:		base := strings.TrimSuffix(name, ext)

github.com/gogits/gogs/routers/repo/repo.go
287:	refName = strings.TrimSuffix(uri, ext)

github.com/openshift/origin/vendor/k8s.io/kubernetes/examples/examples_test.go
162:			name := strings.TrimSuffix(file, ext)

github.com/flynn/flynn/vendor/github.com/jvatic/asset-matrix-go/matrix.go
357:	outputPath = strings.TrimSuffix(outputPath, ext) + "-" + m.cacheBreaker + ext

@rsc
Copy link
Contributor

rsc commented Jun 5, 2018

Thanks for gathering these examples. I looked through a bunch of them. The ones that already have ext when they want to slice it off seem to typically be inserting a string in the middle of the path, between the part before the extension and the extension. That is, they add the extension back. So they need to know what the extension is, so TrimExt by itself does not simplify the surrounding code.

The ones that know the specific extension they want to trim have already checked it and are similarly unhelped by a general TrimExt.

The ones that remain, that blindly strip any extension and return what is left, seem to me incorrect. It's hard for me to see in what context you'd want:

x.tar.gz -> x.tar
x -> x
x.tgz -> x
x.bash -> x
x-1.2.3 -> x-1.2

If you do want that, it's trivial to write, as we established. But I don't see that it's terribly useful for the library to provide it.

Let's leave this one out in the absence of compelling evidence that it has general utility.

@rsc rsc closed this as completed Jun 5, 2018
@kybin
Copy link
Contributor Author

kybin commented Jun 5, 2018 •
edited

@rsc Thanks for the analysing of TrimExt. And I agree.

Actually, what I imagined was filepath.TrimExt(f) + something + filepath.Ext(f).
But it seems little bit inefficient.

Actually I am leaning towards to SplitExt. But I don't want to make a proposal in a hurry again.

@golang golang locked and limited conversation to collaborators Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
4 participants
@rsc @kybin @agnivade @gopherbot