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

[suggestion] Add syntax group for built-in module attributes #533

Open
sodapopcan opened this issue Jul 30, 2020 · 5 comments
Open

[suggestion] Add syntax group for built-in module attributes #533

sodapopcan opened this issue Jul 30, 2020 · 5 comments

Comments

@sodapopcan
Copy link
Collaborator

Would you be open to a PR that adds a syntax group for Elixir's built-in module attributes?

This is a nice-to-have which:

  1. gives immediate feedback that a built-in has been spelled correctly (and indeed exists)
  2. gives immediate feedback when trying to define a built-in as a custom attribute
  3. gives a visual difference between built-ins and user-defined "constants"

Thanks for reviewing and no worries if the answer is no!

@jbodah
Copy link
Collaborator

jbodah commented Jul 30, 2020

This seems reasonable - these are currently mapped to elixirVariable

syn match elixirVariable '@[a-z]\w*'

We coud swap that to elixirModuleAttribute and then add a special syntax for built-ins

My only ask would be that we retain the default mapping to Indentifier

hi def link elixirVariable Identifier

I think folks who want to have the functionality you're suggesting should opt-in to it by setting up a special-case rule in their color scheme

@sodapopcan
Copy link
Collaborator Author

I think folks who want to have the functionality you're suggesting should opt-in to it by setting up a special-case rule in their color scheme

Agreed! Though would you be down for calling it out in the README? Otherwise, unless you use something like scriptease, it's not very discoverable. I do notice that documentation is on the TODO list. I'd be happy to contribute to documentation, or at least some of it. I do love writing docs 🤓

We could swap that to elixirModuleAttribute and then add a special syntax for built-ins

Would there be any worries around breaking peoples' current configs? I was just thinking of introducing elixirBultinVariable. I'm perfectly happy to go with your suggestion, I just thought I'd call it out.

Thanks!

@jbodah
Copy link
Collaborator

jbodah commented Jul 30, 2020

Though would you be down for calling it out in the README?

Yep - that's fine

I'd be happy to contribute to documentation, or at least some of it

That would be amazing if you're into it. Vim :help docs are the eventual goal, but anything would be valuable I think

Would there be any worries around breaking peoples' current configs?

I'm not that concerned. The risk feels low and the impact should be low too (plugin should still work, they can roll back a version if they want to). If it did break anyone it would be after upgrading, and they should (hopefully) be able to see this change and either adapt to it or race a concern here. I think the approach you mention is fine too, but I think there is value to special casing module attributes in general

A bigger breakage might be that some other variable starts with @, but I can't think of any off the top of my head (then again I'm also not writing Elixir very much these days and may be forgetting something)

@sodapopcan
Copy link
Collaborator Author

Apologies for the long delay here.

I have something working but I'm having all sorts of trouble running the tests. I tried both methods. The docker one in particular makes little sense to me. After running & docker-compose build && docker-compose run vim, I get put in a shell attached to the container where no commands are available. Despite using it at work, my docker knowledge is pretty limited. Is there something I'm missing or anything else I can provide that would help? I'm on macOS Catalina.

@sodapopcan
Copy link
Collaborator Author

Ok, after doing some digging I realize I really don't know how to use docker. So after running:

$ docker-compose build && docker-compose run vim

Then running what I assume is the actual command I'm supposed to run:

$ docker-compose run vim bundle exec parallel_rspec spec

I get the following error:

Error response from daemon: OCI runtime create failed: container_linux.go:349: starting container process caused "exec: \"bundle\": executable file not found in $PATH": unknown

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

No branches or pull requests

2 participants