go-mode.el icon indicating copy to clipboard operation
go-mode.el copied to clipboard

Add function to automatically add package header

Open phikal opened this issue 4 years ago • 5 comments

This patch fixes the minor annoyance of having to write out the package name when creating a new file. It scans all other file names and adds a package ... header if all other files share the same package line.

Even with packages with over 100 go files, this function takes less than 0.01 seconds to execute, and even less when compiled.

The current limitations are the only the first 4k byte are used to find a package name, and that it fails if only one file has a different file name (for example in golang.org/x/net/ipv4 there is a file called gen.go that's not part of the package, and thus prevents go-mode-add-package-line from doing anything). These could easily be fixed, but I'm not sure what the correct behavior is from the Go-side of things.

As this feature is intrusive, it's disabled by default. Toggling go-mode-add-package-line automatically adds it to the go-mode-hook.

phikal avatar Apr 28 '20 16:04 phikal

The current limitations are the only the first 4k byte are used to find a package name, and that it fails if only one file has a different file name (for example in golang.org/x/net/ipv4 there is a file called gen.go that's not part of the package, and thus prevents go-mode-add-package-line from doing anything). These could easily be fixed, but I'm not sure what the correct behavior is from the Go-side of things.

I wonder if the correct thing to do would be to pick the package name that occurs most frequently?

sfllaw avatar Sep 24 '21 12:09 sfllaw

Also, it looks like this patch doesn’t seem to handle package foo_test for _test.go files, inside package foo?

sfllaw avatar Sep 24 '21 12:09 sfllaw

I wonder if the correct thing to do would be to pick the package name that occurs most frequently?

That could also be done, but I assume that there could be difficulties when there are an approximately equal number of packages in a directory. It would probably be best to provide a local-safe user option to decide how to do this.

Also, it looks like this patch doesn’t seem to handle package foo_test for _test.go files, inside package foo?

The last update should fix that.

phikal avatar Sep 24 '21 13:09 phikal

I wonder if the correct thing to do would be to pick the package name that occurs most frequently?

That could also be done, but I assume that there could be difficulties when there are an approximately equal number of packages in a directory. It would probably be best to provide a local-safe user option to decide how to do this.

There are four cases that all seem to have a sensible default:

  1. The buffer represents the first .go file in the directory.
    1. In this case, it seems sensible to guess that the package name is the directory name.
  2. There is only one package name that shows up in the directory.
    1. Use that name. This is what your code does now.
  3. There are multiple packages in the directory, but one of them is most frequent.
    1. Use the most frequent name.
  4. There are two package names that have the most .go files in the directory.
    1. If one of the package names matches the directory name, that is probably the best default.
    2. Otherwise, pick one of the most frequent names at random.

sfllaw avatar Sep 24 '21 14:09 sfllaw

FYI gopls provides completion for the package statement now in empty files.

muirdm avatar Sep 24 '21 16:09 muirdm