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

Add golang.org/x/tools/internal/fastwalk #232

Open
dolmen opened this issue Jun 28, 2023 · 5 comments
Open

Add golang.org/x/tools/internal/fastwalk #232

dolmen opened this issue Jun 28, 2023 · 5 comments

Comments

@dolmen
Copy link

dolmen commented Jun 28, 2023

Copy golang.org/x/tools/internal/fastwalk as github.com/rogpeppe/go-internal/fastwalk.

@mvdan
Copy link
Collaborator

mvdan commented Jun 28, 2023

Could you please expand a bit on why you'd want it? At least personally, when I want something faster than filepath.Walk (i.e. with fewer syscalls), I use https://pkg.go.dev/path/filepath#WalkDir. I wonder why that package makes no mention of it.

@mvdan
Copy link
Collaborator

mvdan commented Jun 28, 2023

In fact, fastwalk appears to have zero importers, so perhaps it's entirely unused and should be removed.

@dolmen
Copy link
Author

dolmen commented Jun 29, 2023

  1. I found that fastwalk had been copied in module github.com/mattn/go-zglob a long time ago and that it would be good to upgrade it. But I thought that maintaining/synchronizing that copy would be better done in go-internal, especially if this repo provides tooling to ease such synchronization work. Once fastwalk is available here, I intend to submit a PR to go-zglob that will use it.
  2. golang.org/x/tools/internal/fastwalk is imported by golang.org/x/tools/internal/gopathwalk which is imported by golang.org/x/tools/internal/imports which is imported by golang.org/x/tools/imports and golang.org/x/tools/gopls
  3. I'm ready to do the import job myself and submit a PR.

@mvdan
Copy link
Collaborator

mvdan commented Jun 30, 2023

1 - why is go-zglob not simply using https://pkg.go.dev/path/filepath#WalkDir?

2 - you're right that fastwalk is used by x/tools; pkg.go.dev is wrong about "zero importers". I asked them about it, and they said they wrote this package before filepath.WalkDir existed, and so it's probably obsolete by now. It's just not high priority to do that, I assume, given that it's an internal package.

3 - the work is not the problem :) I mainly do not want to add a package if the need is already met by the standard library. In this case, I'm pretty sure it already is.

@bep
Copy link
Contributor

bep commented Oct 11, 2023

@mvdan I have no strong meaning as to whether fastwalk is useful, though I just woould chime in that fastfalk also

  • scans the file system concurrently
  • optionally follows symlinks

I suspect at least the first bullet is important for gopls.

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

No branches or pull requests

3 participants