andy-blum / drupal-smart-snippets

This extension adds rich language support for Drupal Hooks, Services, and Render Elements to VS Code.
https://marketplace.visualstudio.com/items?itemName=andrewdavidblum.drupal-smart-snippets
MIT License
21 stars 4 forks source link

Use `$this->t` as default for translatable fields #25

Closed tyler36 closed 1 year ago

tyler36 commented 1 year ago

Fixes #24

This PR changes the default translatable value style from t() `$this->t('').

This also prevents DrupalPractice.Objects.GlobalFunction.GlobalFunction warnings from showing when linting with best practises.

Users will be required to import \Drupal\Core\StringTranslation\StringTranslationTrait, if it does not already exist in the class.

17 suggest you would like to kept the extension compatable only with supported Drupal versions and best practises.

andy-blum commented 1 year ago

Thanks for the issue & solution!

The main concern I have with this approach is it requires the code you're writing to be in a class. While that may be your general use case, I know for a fact it is not mine. The majority of the time I'm putting in new elements and would use these snippets I'm doing so in a hook, where $this would be undefined.

I'd also be concerned about novice Drupalers accepting the default and then struggling when $this->t() causes errors.

Let me ping some mentors in the community and get their thoughts on this.

tyler36 commented 1 year ago

@andy-blum thanks for the detailed explaination.

I know of your long time contributions to Drupal (and DDEV) so I knew there was must have been a good reason. I'm fairly new to Drupal and tend to work in module development.

I'd also be concerned about novice Drupalers accepting the default and then struggling when $this->t() causes errors.

This is definately something to be concerned about. Drupal can be overwhelming for novices and we certainly don't want to make it harder.

When I was first writing up the issue, I thought it would be a good case for a configuration option: set your prefered style. I'm not sure VSCode has a hook feature for snippets though.

I'm more than happy to go in what ever direction your experience suggest will work best! Thanks for just considering it. 😄

andy-blum commented 1 year ago

My overly ambitious long-term goals would be to find a way to tap into a language server like PHP Intelephense to be able to create snippets on the fly.

If that were possible, we could definitely push for $this->t() when in classes, as well as pull in hooks/elements/services defined in contrib/custom code.

ultimike commented 1 year ago

I think I'm with @andy-blum on this one - switching to $this->t() will probably confuse newer users when they're writing hooks (including preprocess functions).

I like it the way it is right now, as it defaults to a solution (t()) that works in areas where novice developers typically spend time - in hooks and preprocess functions.

As is discussed above, it would be great if this could depend on the context (in/not in a class) and adjust on the fly. But, in lieu of that capability (currently), I think sticking with just t() is prudent.

-mike

andy-blum commented 1 year ago

Thanks again @tyler36 for the recommendation. For now, I'm opting to leave this as-is.

tyler36 commented 1 year ago

Thank you for taking the time consider it.