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 option to use cwd as package name instead of template #1124

Merged
merged 2 commits into from
Dec 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions autoload/go/template.vim
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
let s:current_file = expand("<sfile>")

function! go#template#create() abort
let l:go_template_use_pkg = get(g:, 'go_template_use_pkg', 0)
let l:root_dir = fnamemodify(s:current_file, ':h:h:h')

let cd = exists('*haslocaldir') && haslocaldir() ? 'lcd ' : 'cd '
Expand All @@ -10,12 +11,19 @@ function! go#template#create() abort
let l:package_name = go#tool#PackageName()

" if we can't figure out any package name(no Go files or non Go package
" files) from the directory create the template
if l:package_name == -1
" files) from the directory create the template or use the cwd
" as the name
if l:package_name == -1 && l:go_template_use_pkg != 1
let l:template_file = get(g:, 'go_template_file', "hello_world.go")
let l:template_path = go#util#Join(l:root_dir, "templates", l:template_file)
exe '0r ' . fnameescape(l:template_path)
$delete _
elseif l:package_name == -1 && l:go_template_use_pkg == 1
" cwd is now the dir of the package
let l:path = fnamemodify(getcwd(), ':t')
Copy link
Owner

Choose a reason for hiding this comment

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

We already have the package name, via l:package_name can you use that instead of using the directory? Because directory and package names can be different in Go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not think I have communicated the intention of my change.

When there are existing files, l:package_name 1= -1, the current behavior is perfect.

When there are not, I do not want to use the template. However, I do want to use the directory name, which, by convention is the same name as the package.

The goal is for it to have it behave the same, whether there are files or not.

I'm sorry if I was unclear before.

Maybe the setting needs a different name to make that clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

g:go_no_template_use_cwd or something like that?

Copy link
Owner

Choose a reason for hiding this comment

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

I understand your changes. Just we should use the name package instead of directory or cwd everywhere. A package might have a different name than a directory.

Do not change the logic as it's correct (just read your comment and it's clear now). But let us change the variable to go_template_use_package or go_template_use_pkg. Does it make sense for you? We don't use cwd or directory nowhere in vim-go because of the reason I've outlined above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean, but I think it would be confusing to a reader (of the docs or code) if the word directory isn't mentioned anywhere.

This change, essentially, guesses that you most likely want to use the name of the directory that a new package is created in as the name of that package.

Using package instead of directory in the docs means that the docs would have to say something like, "use the package name for the name for new packages".

That's confusing because package means two different things in the same sentence.

But something like, "use the directory name of the package as the name for new packages," makes it clear what the two different things are and how they relate.

This would be the first time the directory is mentioned in the docs, but it would also be the first time that it was relevant.

Copy link
Owner

Choose a reason for hiding this comment

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

Using package instead of directory in the docs means that the docs would have to say something like, "use the package name for the name for new packages".

I disagree, it's up to us how to write it down. I went over again and here are the fixed doc sentences:

When a new Go file is created, vim-go automatically fills the buffer content
with a Go code template. By default the template under
`templates/hello_world.go` is used. This can be changed with the
|'g:go_template_file'| setting.

If the new file is created in an already prepopulated package (with other Go
files), in this case a Go code template with only the Go package declaration
(which is automatically determined according to the current package) is added.

To use always the package name instead of the template (`tempaltes/hello_world.go`)
when new Go file is created, enable the |'g:go_template_use_pkg'| setting.

By default it is enabled.
>
  let g:go_template_autocreate = 1
<

and

Specifies that, rather than using a template, the package name is used 
if a new Go file is created. Checkout |'g:go_template_autocreate'| for more 
info. By default the `hello_world.go` file is used.

>
  let g:go_template_use_pkg = 0
<

As you see, it's not complicated and it makes sense to use it this way. It's understandable and there is nowhere the notion of directory. If you just update the docs with the ones above and change the variable name to go_template_use_pkg we're good to go.

let l:content = printf("package %s", l:path)
call append(0, l:content)
$delete _
Copy link
Owner

Choose a reason for hiding this comment

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

Per my comment above, I think you don't need three separate clauses. The following is sufficient:

  if l:go_template_no_file != 1 || l:package_name == -1
    let l:template_file = get(g:, 'go_template_file', "hello_world.go")
    let l:template_path = go#util#Join(l:root_dir, "templates", l:template_file)
    exe '0r ' . fnameescape(l:template_path)
    $delete _
  else
    let l:content = printf("package %s", l:package_name)
    call append(0, l:content)
    $delete _
  endif

Copy link
Owner

Choose a reason for hiding this comment

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

Please neglect this comment.

else
let l:content = printf("package %s", l:package_name)
call append(0, l:content)
Expand Down
12 changes: 12 additions & 0 deletions doc/vim-go.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1442,6 +1442,9 @@ If the new file is created in an already prepopulated package (with other Go
files), in this case a Go code template with only the Go package declaration
(which is automatically determined according to the current package) is added.

To always use the package name instead of the template, enable the
|`g:go_template_use_pkg`| setting.

By default it is enabled.
>
let g:go_template_autocreate = 1
Expand All @@ -1453,6 +1456,15 @@ is created. Checkout |'g:go_template_autocreate'| for more info. By default
the `hello_world.go` file is used.
>
let g:go_template_file = "hello_world.go"
<
*'g:go_template_use_pkg'*

Specifies that, rather than using a template, the package name is used if a new
Go file is created. Checkout |'g:go_template_autocreate'| for more info. By
default the template file specified by |'g:go_template_file'| is used.

>
let g:go_template_use_pkg = 0
<
*'g:go_decls_includes'*

Expand Down