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 language_map option #7

Merged
merged 5 commits into from
Nov 11, 2023
Merged

Add language_map option #7

merged 5 commits into from
Nov 11, 2023

Conversation

bpierre
Copy link
Contributor

@bpierre bpierre commented Aug 29, 2023

This allows to map any vim filetype to a Carbon language.

Example:

require('carbon-now').setup({
  language_map = {
    typescriptreact = "typescript"
  },
})

Or with a function:

require('carbon-now').setup({
  language_map = function(filetype)
    if (filetype == "typescriptreact") then
      return "typescript"
    end
  end
})

A map of known filetypes has also been added, only containing typescriptreact and javascriptreact for now.

@ellisonleao
Copy link
Owner

@bpierre thanks for the PR, but isn't vim.filetype resolving that same problem?

@bpierre
Copy link
Contributor Author

bpierre commented Oct 30, 2023

Not completely sure but I think vim.filetype only allows to define file pattern => vim filetype mappings, while this PR allows to define a vim filetype => carbon filetype mappings (with a fallback to the vim filetype).

@ellisonleao
Copy link
Owner

@bpierre got it. How about making that change only internally. I still think if we make it as a config can lead to a lot user mistakes. How about creating a internal mapping and making that same from => to logic for those special cases?

@bpierre
Copy link
Contributor Author

bpierre commented Nov 1, 2023

That could be a solution yes, though I think it could lead to issues for some people since Neovim filetypes are arbitrary (even though users, plugins etc. generally tend to agree on the same names). Also Carbon could add new languages at any point, having this option would allow to use them without having to wait for a carbon-now.vim update.

Another solution could also be to put language_map inside an advanced field to make it clear that it shouldn’t be needed in most cases:

{
  advanced = {
    language_map = {},
    open_cmd = "", -- open_cmd could be considered advanced too if #8 gets merged
    base_url = "" -- base_url could also be considered an advanced use case
  }
}

Another one would be to put an “Advanced” section in the documentation to separate language_map and maybe also base_url / open_cmd from the theming options?

Let me know what would be your preferred solution and I’ll make the changes.

@ellisonleao
Copy link
Owner

@bpierre not a big fan of too nested configs, but i get the point. Let's keep it simple for now and just make the internal changes (i am ok with checking new languages in carbon if needed)

@bpierre
Copy link
Contributor Author

bpierre commented Nov 1, 2023

Cool I’ll make the change 👍

@bpierre
Copy link
Contributor Author

bpierre commented Nov 10, 2023

@ellisonleao done 🤗

@ellisonleao ellisonleao merged commit 84b15d0 into ellisonleao:main Nov 11, 2023
2 of 3 checks passed
@bpierre bpierre deleted the language-map branch November 16, 2023 18:43
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