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

feat: add ftdetect for .props #186

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

molostovvs
Copy link
Contributor

When working with Directory.Build.props or Directory.Packages.props files, it is annoying that the file type is not detected, which makes commenting plugins and syntax highlighting not work.

GustavEikaas
GustavEikaas previously approved these changes Dec 7, 2024
Copy link
Owner

@GustavEikaas GustavEikaas left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for making a PR. I have also missed this previously!❤️❤️

I do worry that this might interfere with other non-dotnet scenarios. We should consider the following.

  1. Could we use the is_dotnet_project() function upon VimEnter to enable this?
  2. Could we add a property to options for enabling/disabling this, in case it interferes with other scenarios

Thoughts? We could also merge it and wait for possible issues on this before implementing some variant of the options above

@@ -190,6 +190,9 @@ M.setup = function(opts)
require("easy-dotnet.fs-mappings").add_test_signs()
end

if merged_opts.enablenable_filetypes == true then
Copy link
Owner

Choose a reason for hiding this comment

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

enable_filetypes

@molostovvs
Copy link
Contributor Author

Hi! Sorry I didn't reply to the comment. the original implementation was taken from go.nvim (where just filetype.vim is used), but the feature toggle approach looks better.

Now I'm not sure how to properly implement the project type check once to create autocmd once, and how not to set filetype in the buffer without closing it (i.e. avoid setting filetype when :e is used).

@GustavEikaas
Copy link
Owner

GustavEikaas commented Dec 9, 2024

Now I'm not sure how to properly implement the project type check once to create autocmd once, and how not to set filetype in the buffer without closing it (i.e. avoid setting filetype when :e is used).

Im on vacation without a computer until Friday so probably wont be able to merge before then. Hmm have you tried BufReadPost? Is there any drawbacks or side-effects when setting filetype multiple times?

@GustavEikaas
Copy link
Owner

GustavEikaas commented Dec 14, 2024

Havent tested extensively but this seems to work for me, only creates the autocmd once and only sets the filetype once per buffer.

local M = {}

M.enable_filetypes = function()
  vim.api.nvim_create_autocmd({ "BufReadPost" }, {
    pattern = "*.props",
    group = vim.api.nvim_create_augroup("solution_props", { clear = true }),
    callback = function()
      vim.bo.filetype = "xml"
    end,
  })
end

return M

@GustavEikaas
Copy link
Owner

GustavEikaas commented Dec 14, 2024

Messed around in some other vim plugins today and came across this. Seems to be working and be the best and easiest solution

local M = {}

M.enable_filetypes = function()
  vim.filetype.add({
    extension = {
      props = "xml",
    },
  })
end

return M

@GustavEikaas
Copy link
Owner

Didnt mean to hijack your PR. Let me know if you disagree with the changes

@GustavEikaas
Copy link
Owner

We should extend this to also add xml for slnx, #220

@GustavEikaas
Copy link
Owner

I upstreamed these file type changes to the official vim repository.

Msbuild: 32b7e3a
Slnx: 3b3318b

We can still merge this PR as it will take a while for this change to propagate to everyones nvim installation

@GustavEikaas
Copy link
Owner

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

Successfully merging this pull request may close these issues.

2 participants