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

Contextual parsing of Rails controllers #565

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davidwessman
Copy link
Collaborator

@davidwessman davidwessman commented May 7, 2024

  • Adding a Parser based on Prism.

TODO:

  • Handle comments
  • Add magic comment to skip parsing file
  • Handle all arguments to translation method
  • Handle Rails-model calls
  • Handle more before_actions, or warn that they cannot be processed

@davidwessman davidwessman force-pushed the controller-contextual-parsing branch from 36d0316 to fadc07d Compare May 8, 2024 05:28
@glebm
Copy link
Owner

glebm commented May 10, 2024

We already have a Ruby parser dependency, consider reusing it rather than adding another one.

@davidwessman
Copy link
Collaborator Author

We already have a Ruby parser dependency, consider reusing it rather than adding another one.

Prism is a standard ruby gem and is included by default from Ruby 3.3 and can be installed from Ruby 3.0.
I think we should investigate moving away from parser towards it.

I will do this experiment in Prism and then I can rewrite it to use the old parser.

When parsing controllers I would like to return a key or a fallback, e.g. if there is a relative key in a method name I would like the key with either the method name included or with just the controller namespace.

class EventsController < ApplicationController
  def show
    t(".relative")
  end
end

Then I would like it to first check for events.show.relative, but if that does not exist, fallback to events.relative.
Do you have any idea where to start with adding such behaviour?

@davidwessman davidwessman force-pushed the controller-contextual-parsing branch 2 times, most recently from a325228 to 1fb6f4b Compare May 10, 2024 05:53
@glebm
Copy link
Owner

glebm commented May 10, 2024

I don't mind dropping support for older rubies, but does Prism support JRuby?
If so, switching to Prism throughout might be a good idea

Then I would like it to first check for events.show.relative, but if that does not exist, fallback to events.relative.

Absolute keys are resolved by calling absolute_key:

# @param key [String] relative i18n key (starts with a .)
# @param path [String] path to the file containing the key
# @param roots [Array<String>] paths to relative roots
# @param calling_method [#call, Symbol, String, false, nil]
# @return [String] absolute version of the key
def absolute_key(key, path, roots: config[:relative_roots],

Sounds like the code calling absolute_key would need to have access to the data:

# I18n data provider
# @see I18n::Tasks::Data::FileSystem
def data

@glebm
Copy link
Owner

glebm commented May 10, 2024

Looks like Prism on JRuby is still a work-in-progress: http://blog.jruby.org/2024/02/jruby-prism-parser

@davidwessman davidwessman force-pushed the controller-contextual-parsing branch 2 times, most recently from 6946055 to 305b747 Compare May 11, 2024 10:03
@jaredmoody
Copy link

+1 for Prism! The speed gains rubocop is getting from using it are 👀

@davidwessman
Copy link
Collaborator Author

Looks like Prism on JRuby is still a work-in-progress: http://blog.jruby.org/2024/02/jruby-prism-parser

I think this means that JRuby are working on replacing their parser with Prism, for their compilation.
Prism can still handle all syntax that JRuby supports and can run in JRuby, so for our use-case it would be okay.

@davidwessman davidwessman force-pushed the controller-contextual-parsing branch 2 times, most recently from 244bf22 to 5d7ae4e Compare May 18, 2024 19:18
@davidwessman davidwessman force-pushed the controller-contextual-parsing branch from 5d7ae4e to 2018d84 Compare May 19, 2024 06:22
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.

None yet

3 participants