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

Improve go.mod filetype detection #1934

Merged
merged 1 commit into from Sep 3, 2018

Conversation

2 participants
@Carpetsmoker
Collaborator

Carpetsmoker commented Aug 31, 2018

Comments are allowed in the go.mod file, so previously:

// Document stuff!

module foo/bar

require "other/pkg"

Would not get detected properly.

Also tweak regexp slightly: module must be followed by a space and at
least one character.

Improve go.mod filetype detection
Comments are allowed in the go.mod file, so previously:

	// Document stuff!

	module foo/bar

	require "other/pkg"

Would not get detected properly.

Also tweak regexp slightly: `module` must be followed by a space and at
least one character.
@Carpetsmoker

This comment has been minimized.

Show comment
Hide comment
@Carpetsmoker

Carpetsmoker Aug 31, 2018

Collaborator

There is a second problem, by the way. The modsim3 is still loaded, meaning that settings, highlights, etc. are loaded from that as well, which may be unexpected. I'm not sure how to fix/prevent that.

There is only syntax/modsim3.vim, which doesn't do anything strange. Since this is a very old/obscure tech, I don't expect any changes, and this should be fine (but I was curious if it could be done "more correct").

Collaborator

Carpetsmoker commented Aug 31, 2018

There is a second problem, by the way. The modsim3 is still loaded, meaning that settings, highlights, etc. are loaded from that as well, which may be unexpected. I'm not sure how to fix/prevent that.

There is only syntax/modsim3.vim, which doesn't do anything strange. Since this is a very old/obscure tech, I don't expect any changes, and this should be fine (but I was curious if it could be done "more correct").

set filetype=gomod
endif
break

This comment has been minimized.

@bhcleek

bhcleek Aug 31, 2018

Collaborator

Should this break be moved into the previous if block?

@bhcleek

bhcleek Aug 31, 2018

Collaborator

Should this break be moved into the previous if block?

This comment has been minimized.

@Carpetsmoker

Carpetsmoker Aug 31, 2018

Collaborator

No, once we see a non-blank line or a line that is not a non-comment we don't need to look any further. module pkg must the first directive.

@Carpetsmoker

Carpetsmoker Aug 31, 2018

Collaborator

No, once we see a non-blank line or a line that is not a non-comment we don't need to look any further. module pkg must the first directive.

@bhcleek

This comment has been minimized.

Show comment
Hide comment
@bhcleek

bhcleek Sep 1, 2018

Collaborator

lgtm

Collaborator

bhcleek commented Sep 1, 2018

lgtm

@bhcleek

This comment has been minimized.

Show comment
Hide comment
@bhcleek

bhcleek Sep 1, 2018

Collaborator

Can you use syntax clear to clear the syntax of modsim3 ?

Collaborator

bhcleek commented Sep 1, 2018

Can you use syntax clear to clear the syntax of modsim3 ?

@Carpetsmoker

This comment has been minimized.

Show comment
Hide comment
@Carpetsmoker

Carpetsmoker Sep 1, 2018

Collaborator

Changing the syntax automatically clears the syntax, just not the highlight groups. hi clear doesn't seem to do that.

The real danger is mostly from settings that some filetypes set (e.g. tabstop, expandtab, comments, etc.).

Collaborator

Carpetsmoker commented Sep 1, 2018

Changing the syntax automatically clears the syntax, just not the highlight groups. hi clear doesn't seem to do that.

The real danger is mostly from settings that some filetypes set (e.g. tabstop, expandtab, comments, etc.).

@bhcleek

This comment has been minimized.

Show comment
Hide comment
@bhcleek

bhcleek Sep 1, 2018

Collaborator

I think that simply adding adding a ! to the au instruction do what you're looking for in this case.

Collaborator

bhcleek commented Sep 1, 2018

I think that simply adding adding a ! to the au instruction do what you're looking for in this case.

@bhcleek

This comment has been minimized.

Show comment
Hide comment
@bhcleek

bhcleek Sep 1, 2018

Collaborator

Since we're using go.mod as the pattern, we probably need to clear the existing groups first by having an explicit au! BufNewFile,BufRead *.mod before our au BufNewFile,BufRead go.mod

Collaborator

bhcleek commented Sep 1, 2018

Since we're using go.mod as the pattern, we probably need to clear the existing groups first by having an explicit au! BufNewFile,BufRead *.mod before our au BufNewFile,BufRead go.mod

@Carpetsmoker

This comment has been minimized.

Show comment
Hide comment
@Carpetsmoker

Carpetsmoker Sep 1, 2018

Collaborator

The filetype is already set, so I don't see how clearing the autogroups will undo that?

Ideally Vim's filetype.vim should be fixed/amended; I'll make a PR in the Vim repo, too.

Collaborator

Carpetsmoker commented Sep 1, 2018

The filetype is already set, so I don't see how clearing the autogroups will undo that?

Ideally Vim's filetype.vim should be fixed/amended; I'll make a PR in the Vim repo, too.

@bhcleek

This comment has been minimized.

Show comment
Hide comment
@bhcleek

bhcleek Sep 1, 2018

Collaborator

The filetype is already set because the autogroup's command was executed. If you clear the autogroup that filetype.vim sets up, then its command (which ends up setting the filetype), won't be executed at all....

Collaborator

bhcleek commented Sep 1, 2018

The filetype is already set because the autogroup's command was executed. If you clear the autogroup that filetype.vim sets up, then its command (which ends up setting the filetype), won't be executed at all....

@Carpetsmoker Carpetsmoker merged commit f7c771b into fatih:master Sep 3, 2018

2 of 3 checks passed

codecov/patch 22.22% of diff hit (target 29.99%)
Details
codecov/project 29.96% (-0.03%) compared to 0b21790
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Carpetsmoker Carpetsmoker deleted the Carpetsmoker:gomod-ft branch Sep 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment