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 projections.json for Dispatch.vim support #405

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

calebhearth
Copy link
Contributor

Rails.vim allows gems to provide their own projections compatible with
Dispatch.vim by placing them at lib/rails/projections.json. This
configuration allows:

  • :A from a factory to alternate to the test file
  • :Efactory (and :{S,V,T}factory) to navigate to or create a new factory
  • highlights FactoryBot macros as Ruby macros
  • Sets up :{E,S,V,T}unittest for factories => model specs

To use this, you'll need Rails.vim, Bundler.vim, and Dispatch.vim
plugins.

See :help rails-projections or the file on
GitHub

for more clarity on what specifically each config key is doing.

Inspired and updated from @henrik's
Gist

@tpope
Copy link

tpope commented Sep 19, 2022

Found this via https://twitter.com/caleb_hearth/status/1567269197545340928, wherein the recent removal of this rails.vim feature, and the possibility of bringing it back, are discussed. I've tentatively done so.

Note that rails.vim has partial support for FactoryBot built-in by way of :Efixtures. I did this because :Efixtures is a special case command that isn't possible to extend, and because conceptually it's a pretty clean fit, filling the same use case. This also covers other navigational uses like :A. Until today it also forced the default task to be db:fixtures:load; I just fixed that. What I didn't take responsibility for is syntax highlighting and the template, as there are official interfaces for doing that externally.

That said, I think any attempt at an external implementation should attempt to do as much as possible, even if it's redundant, for future-proofing. I bring it up because it might explain some of the oversights below.

Issues with this PR:

  • The title of this PR and the commit message reference the wrong plugin.
  • The naming scheme used by the generators provided by factory_bot_rails is plural, they use the name of the table. These projections, however, assume a singular factory name. "affinity" should be "collection" and the {} expansions should be {singular}.
  • This only supports spec/fixtures. Supporting text/fixtures can be achieved by duplication. The "test" key only needs to reference the corresponding test suite's file. (test/unit/ is presumably older than FactoryBot supports and consequently could be dropped.)
  • The preferred format for the template is an array of lines, not a string with newlines. This serves to disambiguate it from a future filename variant.
  • The list of highlighted keywords barely scratches the surface. A quick check of GETTING_STARTED reveals that add_attribute, association, after, before, callback, initialize_with, traits_for_enum, to_create, and skip_create are omitted, and I have no reason to believe that list is exhaustive.

That last one raises a question: Is there any way the list of highlighted keywords can be kept up-to-date automatically? If this list is going to fall out of date, it's probably better not to ship it at all. Nothing worse than syntax highlighting that lies to you. (If the API is fully matured and no longer gaining new methods, that would also render this moot.)

@calebhearth
Copy link
Contributor Author

Thanks @tpope for the input here! Lots of good stuff, some sounds like easy fixes and some like discussions. I have a tough schedule this week but I'm happy to circle back on this as soon as my schedule frees up, hopefully next week (kiddo is sick and home from day care, so I'm parenting a lot more than usual)

@calebhearth
Copy link
Contributor Author

  • The list of highlighted keywords barely scratches the surface. A quick check of GETTING_STARTED reveals that add_attribute, association, after, before, callback, initialize_with, traits_for_enum, to_create, and skip_create are omitted, and I have no reason to believe that list is exhaustive.

That last one raises a question: Is there any way the list of highlighted keywords can be kept up-to-date automatically? If this list is going to fall out of date, it's probably better not to ship it at all. Nothing worse than syntax highlighting that lies to you. (If the API is fully matured and no longer gaining new methods, that would also render this moot.)

Anyone at thoughtbot have an opinion on this one? If not, I'm inclined to agree that it's better to omit. I think all it will do is add some highlights to certain methods, which doesn't add value commensurate with the maintenance burden.

@calebhearth
Copy link
Contributor Author

I guess I had some time.

@mike-burns
Copy link
Contributor

The thing I'm looking at here is that only the vim-projectionist project loads these files -- other implementations such as vscode-alternate-file and the command-line projectionist project only look for .projectionist.json files in the current or parent directories.

So this would be something for only the (Neo)Vim users. That said, I'm one of those.

How much work will this be to maintain? It looks like something I can set and forget -- is that actually true?

Rails.vim allows gems to provide their own projections compatible with
Projectionist.vim by placing them at lib/rails/projections.json. This
configuration allows:

- :A from a factory to alternate to the test file
- :Efactory (and :{S,V,T}factory) to navigate to or create a new factory
- highlights FactoryBot macros as Ruby macros
- Sets up :{E,S,V,T}unittest for factories => model specs

To use this, you'll need Rails.vim, Bundler.vim, and Projectionist.vim
plugins.

See `:help rails-projections` or [the file on
GitHub](https://github.com/tpope/vim-rails/blob/6bc0c7826d68f8c44c8347a3012aa79ade4f0a22/doc/rails.txt#L611-L726)
for more clarity on what specifically each config key is doing.

Inspired and updated from [@henrik's
Gist](https://gist.github.com/henrik/5676109#file-config-projections-json-L10-L15)
@mike-burns mike-burns merged commit 659d74a into thoughtbot:main Sep 8, 2023
22 checks passed
@mike-burns
Copy link
Contributor

Alright, merged. Thank you for this @calebhearth , and thanks to @tpope for the suggested improvements (and for writing projectionist in the first place).

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.

4 participants