Fix aliases for stylesheet_engine option in Generators: #38813
Conversation
If using one-dash the alias for stylesheet_engine wont work. This patch change the `se` alias to two dash makes work for this option alias.
roramirez
force-pushed
the
stylesheet_engine_alias
branch
from
512f95d to
a790251
Compare
|
Does the same thing apply to javascript_engine |
|
When i run |
|
Hi @p8 !This change should applied for the aliases with more than one character. So at this time for you example this wont work
If this MR is accept I'll can do for other aliases. |
|
@roramirez Ah, sorry, |
|
:) |
| def test_scaffold_generator_alias_nocss_stylesheets_engine | ||
| output = run_generator [ "posts", "--se=NOCSS" ] | ||
| assert_no_file "app/assets/stylesheets/posts.css" | ||
| assert_match(/NOCSS \[not found\]/, output) | ||
| end |
There was a problem hiding this comment.
I'm not sure this test adds anything to this PR. It's also the only one in ScaffoldGeneratorTest that tests for "not found".
There was a problem hiding this comment.
Is a reverse way for test_scaffold_generator_alias_css_stylesheets_engine I'll try to use a default scss but for this test don't have sense.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
@jonathanhefner This is a tiny fix. Basically generator aliases with two-letters (se, ss, je) require a double dash but are shown with single dash. |
|
Thank you for the pull request. I think a better fix might be to ensure all single-dash options use single letters, since that is conventionally expected. However, I also question how much utility some of these aliases provide. So if it's too difficult to think of sensible single-letter replacements, my inclination would be to remove them altogether. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
I agree with @jonathanhefner and I believe the best thing would be to remove those aliases. Maybe we should go further and even change the |
|
@roramirez are you interested in working on this? |
|
Sure, I'll remove them a push a new commit... I'm not really sure is move into a new PR, amend the current commit or add new one, what do you think? |
|
You call. A new PR could be easier to deal with |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
There a issues is use more than one character to use as aliases https://github.com/erikhuda/thor/blob/a5cbed8/lib/thor/base.rb#L307 So, for this change the convention is remove the two letters aliases #38813
There a issues is use more than one character to use as aliases https://github.com/erikhuda/thor/blob/a5cbed8/lib/thor/base.rb#L307 So, for this change the convention is remove the two letters aliases rails#38813
If using one-dash the alias for stylesheet_engine wont work.
This patch change the
sealias to two dash makes work for this option alias.