`ember new --lang` by hergaiety · Pull Request #635 · emberjs/rfcs

hergaiety · 2020-05-29T20:28:04Z

Rendered
Candidate Implementation

Original draft for preserving comment history

Co-authored-by: Jamie White <jameswhite@salesforce.com>

Co-authored-by: Melanie Sumner <melaniersumner@gmail.com>

Gaurav0 · 2020-05-29T20:50:40Z

What happens if no --lang flag is passed?

Can a default lang setting be set in ~/.ember-cli or ~/.ember-cli.js?

chancancode · 2020-05-29T21:42:01Z

+Unrecognized language subtag, "uk".
+```
+
+#### No argument passed


IMO, this is just a "bug" / limitation of the command line argument parser used by Ember CLI. This RFC introduces a new named argument that takes a string value. If you didn't pass a value, it should be an error. If a second named argument follows it, it should not be parsed as the value. Basically it should work the same as --blueprint or anything else like it.

I think it's fine to implement whatever workaround is needed, but I'm not sure it needs to be included in the RFC. It's fine to include it for additional information, I suppose, but it shouldn't be considered "normative". Once we fix any bugs or limitations in the arguments parsing, this section will become obsolete.

TL;DR I don't think the RFC should be or is propose a permeant special error handling of this particular CLI argument.

I think it's fine to implement whatever workaround is needed, but I'm not sure it needs to be included in the RFC.

Ya, agreed (RE: this being a bug).

@chancancode @rwjblue I'm sure we're willing to incorporate feedback for what in ember-cli is a bug and what's intended to be there- I mostly assume that the things that are there are intended to be there unless there are code comments that indicate otherwise.

chancancode · 2020-05-29T21:54:25Z

+  See `ember <command> help` for more information.
+```
+
+#### Common Misunderstandings: Programming Languages


IMO this is just a special case of the "Invalid Language Codes". In implementation speak:

let lang; try { // validateLangFlag checks against the ISO 3166 list lang = validateLangFlag(langFlag); } catch(e) { if (isProgrammingLanguage(langFlag)) { throw new Error("specific error message about how this flag is not doing what you think"); } else { throw new Error("generic error message about how this is not a valid language"); } }

In the unlikely case that they overlap (that it is both a valid language and a programming language) I think it's okay to print a warning, but probably shouldn't fail the build and disallow what is technical a valid input.

chancancode · 2020-05-29T22:00:32Z

+An error with the `--lang` flag returned the following message:
+  Detected lang specification starting with command flag `-`.
+  Is `--skip-git` meant to be an ember-cli command option?
+  This issue is likely caused by using the `--lang` flag without a specification.


Again I think this shouldn't be in the normative part of the RFC anyway, but IMO this is working around a bug in the argument parsing, so exposing this much information about the limitation/bug/workaround in our argument parsing is more confusing to the user than it is helpful.

IMO, I think both of these cases should show something short like "The --lang flag requires a valid language code, see ember <command> help for more information.". In the second example, the --disable-analytics` bit is extra confusing, because it's not part of what the user typed and definitely a quirk of our internal argument parsing implementation.

Concretely, if the parsed value starts with a dash, I think it's just a bug in the parsing and we can show a message about the missing language code. Otherwise, we just validate against the ISO list.

+1. I think a more consistent way to do this is when parse fails, print a message and either a descriptive help message with what the flag is for, or a link to a URL that has "common misunderstandings" if it is necessary. Trying to guess user intention is probably not going to scale very well.

chancancode · 2020-05-29T22:04:20Z

What happens if no --lang flag is passed?

That would be the current/default behavior (no lang attribute in index.html), which is useful if you are going to deal with that some other way (via an internationalization plugin, etc).

Can a default lang setting be set in ~/.ember-cli or ~/.ember-cli.js?

Like any other Ember CLI options, it can be set in ~/.ember-cli, which the caveat that it will be passed to all Ember CLI commands, which fine as long as nothing else uses it for a conflicting meaning. It's an unfortunate limitation of the current Ember CLI implementation.

chancancode · 2020-05-29T22:05:29Z

Thank you everyone for doing the investigation and putting together! It's very well written.

Gaurav0 · 2020-05-29T22:42:53Z

That would be the current/default behavior (no lang attribute in index.html), which is useful if you are going to deal with that some other way (via an internationalization plugin, etc)

I don't think this is adequate. Ember needs to become accessible by default. Either a lang should be required or a default language selected.

MelSumner · 2020-05-30T00:26:37Z

What happens if no --lang flag is passed?

Nothing will happen. It will be the same as it is now.

Can a default lang setting be set in ~/.ember-cli or ~/.ember-cli.js?

Why? It needs to go in the index.html file on the HTML tag.

MelSumner · 2020-05-30T00:30:13Z

That would be the current/default behavior (no lang attribute in index.html), which is useful if you are going to deal with that some other way (via an internationalization plugin, etc)

I don't think this is adequate. Ember needs to become accessible by default. Either a lang should be required or a default language selected.

It’s backwards compatible & I wouldn’t suggest anything else for the MVP.

chancancode · 2020-05-30T00:41:01Z

I don't think this is adequate. Ember needs to become accessible by default. Either a lang should be required or a default language selected.

I agree with the general sentiment but this RFC is a step in the right direction and doesn't need to fix everything at once. This allows some developers (who are aware of the issue, and only support a single language in the app) to start using the feature, allows us to the start teaching it in the tutorial, etc. And subject to the caveats I mentioned above, you can even set it as a default in your ~/.ember-cli.

In general, there a lot of other things like this that doesn't have a good, global default answer for everyone, but may have a good enough default answer for you. For example, in the case of ember new:

What LICENSE file to generate, or do you want one at all? (Set private field in package.json?)
Do you want us to generate a code of conduct? If so, which one to use? Or is this a private project, so it's not needed since it's already covered by internal company guidelines? Or do you want to delegate to the Ember community guidelines?
Technical things like what CI service do you want to use? Do you want to use TypeScript or not? What test framework?

For a lot of these questions, there aren't necessarily one good answer for a lot of these questions, and right now our infrastructure is not really setup to accommodate that kind of stuff. For the lang attribute in particular, setting a wrong default value could cause issues too, as mentioned in the RFC, and it would be quite heavy handed and presumptuous of us, IMO, to pick en or en-us as a default here.

We need a more general solution for these kind of things, probably something along the lines of an interactive ember new that allows you to state your preference for some of these things and remembers it. We also need a better way to store and pass along these values, as the caveats of the ~/.ember-cli "flash namespace" is really hindering us here.

A lot of these things are already in progress or on different people's radar. However, we don't have to block on those improvements to start making progress here. This is a good feature to add now, and when those other things get sorted out, they will just be implemented on top of what we started here.

locks · 2020-05-30T10:32:30Z

+
+
+
+## How we teach this


Great work on this RFC! I enjoy how well reasoned it is and its focus.

For that reason, I would like to see the "How we teach this" section expanded with some of the knowledge in the motivation section. The "Testing it out" was particularly illumating to me as a content creator in understanding the implications of the lang attribute.

We have a Language attribute section in the Ember Guides that could also be updated to mention the new flag?

@locks yes, that seems correct. I’ll work on it. Thanks for the feedback!

abhilashlr · 2020-05-31T07:55:10Z

Amazing work on this RFC :)

Question: Would there be an error if the --lang flag value is en-us instead of en-US?

chriskrycho · 2020-06-01T04:36:57Z

Can a default lang setting be set in ~/.ember-cli or ~/.ember-cli.js?

Why? It needs to go in the index.html file on the HTML tag.

The ~/.ember-cli and ~/.ember-cli.js files allow users to set default values of arguments for Ember CLI on their machines: if you have lang: 'fr' set, you never have to pass --lang='fr' explicitly. An example use case here: a team which always wants to default the language to fr could set their dev machine config up so that every new Ember dev gets it configured out of the box, and any new app they spin up always has it configured correctly.

josephdsumner · 2020-06-01T14:59:53Z

@abhilashlr

Amazing work on this RFC :)

Question: Would there be an error if the --lang flag value is en-us instead of en-US?

Thanks! And no, there shouldn’t be an error for that example (or others related to case sensitivity). Here’s the relevant unit tests from the candidate:

@locks

Update to address feedback from @locks

rajasegar · 2020-06-03T16:20:05Z

Awesome job on the RFC, thanks a lot for putting this up.

rwjblue · 2020-06-08T14:32:54Z

After discussion at Friday's core team meeting, we decided to move this into final comment period.

🗣️💬⌛

Gaurav0 · 2020-06-09T18:07:01Z

I don't think this is adequate. Ember needs to become accessible by default. Either a lang should be required or a default language selected.

I agree with the general sentiment but this RFC is a step in the right direction and doesn't need to fix everything at once. This allows some developers (who are aware of the issue, and only support a single language in the app) to start using the feature, allows us to the start teaching it in the tutorial, etc. And subject to the caveats I mentioned above, you can even set it as a default in your ~/.ember-cli.

i agree, it is a step forward and I don't want to hold this up. I withdraw my objection.

luxzeitlos · 2020-06-09T20:39:43Z

So should an app that uses ember-intl use this flag and define the "default" language or not?

hergaiety · 2020-06-10T15:24:34Z

@luxferresum ember-intl sets the html tag's lang attribute anyway ember-intl/ember-intl#649, so if you plan to use that addon then you don't need to use this flag necessarily but it also won't conflict. So you can't go wrong here, is the idea.

As this relates to ember-intl

The popular localization library ember-intl does not conflict with the addition of this new Ember CLI flag. The addition of this flag offers some out of the box support where it was previously missing for new Ember apps. It is still recommended that globalized apps leverage ember-intl.

rwjblue · 2020-06-12T22:43:37Z

We discussed this in todays core team meeting and are good to go here 🥰

sandstrom · 2020-06-24T16:57:53Z

If the flag is used, would it make sense to output a console message on ember new telling people about ember-intl specifically, or to suggest a search in ember-addons (or similar website) to look for an addon with more i18n capabilities?

Right now it doesn't really do anything except setting a simple html attribute, so people may be disappointed that "it doesn't do more". That said, I don't think it needs to do more, internationalization is probably a better fit for an addon regardless. And ember-intl is an excellent addon!

Ava Wroten and others added 30 commits April 29, 2020 10:26

First pre-draft commit to begin collaboration of ember-new-lang RFC

805bfa6

Added more information to Motivation section

5fd0fb3

Corrected link to issue

e05f359

More motivation bullet points

2bdffa3

Filled out How we teach this

4e885c2

Removed template prompts from Motivation section

024362e

Added an unresolved question

86a7624

Added a drawback

3dbfe25

Classic "do nothing" alternative for now

7b93b87

Further defines the motivation GO and NO GO's

e66ba23

Fix motivation typo

61d6e3d

Update text/0000-ember-new-lang.md

52b9069

Co-authored-by: Jamie White <jameswhite@salesforce.com>

Update text/0000-ember-new-lang.md

3ecfcb3

Co-authored-by: Jamie White <jameswhite@salesforce.com>

Update text/0000-ember-new-lang.md

9b7daa5

Co-authored-by: Jamie White <jameswhite@salesforce.com>

Unordered list drawbacks

eafdfaa

Detailed design

fb0fea3

Added some text before the help command

b1fb15f

Cleaned up common misunderstandings copy

40fb2f5

Converted from --language to --lang

87e8597

Trimmed help snippet

d6e69a4

Updated candidate link

7f79e1f

Added note for why we're chosing --lang

7a13473

Notes For and Against setting default lang: en-US

a36d067

Update Summary section per review

867cf23

Fix typo

e7e41a2

Phrasing

f51a26a

Update Motivation section per review

eb6d8c3

Add reference to lang definition in HTML spec per review

84ddb6b

Update text/0000-ember-new-lang.md per review

6ee2d07

Co-authored-by: Melanie Sumner <melaniersumner@gmail.com>

Update text/0000-ember-new-lang.md per review

3640318

Co-authored-by: Melanie Sumner <melaniersumner@gmail.com>

Update 0000-ember-new-lang.md

7227920

chancancode reviewed May 29, 2020

View reviewed changes

locks reviewed May 30, 2020

View reviewed changes

amyrlam mentioned this pull request May 30, 2020

The Ember Times No. 151 - June 5th 2020 ember-learn/ember-blog#666

Merged

13 tasks

Update 0000-ember-new-lang.md

3d358bd

Update to address feedback from @locks

rwjblue added the Final Comment Period label Jun 8, 2020

rwjblue changed the title ~~Ember New Lang~~ ember new --lang Jun 8, 2020

Update and rename 0000-ember-new-lang.md to 0635-ember-new-lang.md

798342d

rwjblue merged commit ed51170 into emberjs:master Jun 12, 2020

josephdsumner mentioned this pull request Jun 13, 2020

RFC #635: ember new --lang ember-cli/ember-cli#9259

Merged

12 tasks

ijlee2 mentioned this pull request Jun 30, 2020

Adjustments to A11y Lang Attribute section ember-learn/guides-source#1467

Merged

Uh oh!

Conversation

hergaiety commented May 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gaurav0 commented May 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chancancode May 29, 2020

Choose a reason for hiding this comment

Uh oh!

rwjblue May 30, 2020

Choose a reason for hiding this comment

Uh oh!

MelSumner Jun 1, 2020

Choose a reason for hiding this comment

Uh oh!

chancancode May 29, 2020

Choose a reason for hiding this comment

Uh oh!

chancancode May 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mehulkar May 29, 2020

Choose a reason for hiding this comment

Uh oh!

chancancode commented May 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chancancode commented May 29, 2020

Uh oh!

Gaurav0 commented May 29, 2020

Uh oh!

MelSumner commented May 30, 2020

Uh oh!

MelSumner commented May 30, 2020

Uh oh!

chancancode commented May 30, 2020

Uh oh!

locks May 30, 2020

Choose a reason for hiding this comment

Uh oh!

MelSumner Jun 1, 2020

Choose a reason for hiding this comment

Uh oh!

abhilashlr commented May 31, 2020

Uh oh!

chriskrycho commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josephdsumner commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajasegar commented Jun 3, 2020

Uh oh!

rwjblue commented Jun 8, 2020

Uh oh!

Gaurav0 commented Jun 9, 2020

Uh oh!

luxzeitlos commented Jun 9, 2020

Uh oh!

hergaiety commented Jun 10, 2020

As this relates to ember-intl

Uh oh!

rwjblue commented Jun 12, 2020

Uh oh!

sandstrom commented Jun 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

hergaiety commented May 29, 2020 •

edited

Loading

Gaurav0 commented May 29, 2020 •

edited

Loading

chancancode May 29, 2020 •

edited

Loading

chancancode commented May 29, 2020 •

edited

Loading

chriskrycho commented Jun 1, 2020 •

edited

Loading

josephdsumner commented Jun 1, 2020 •

edited

Loading

sandstrom commented Jun 24, 2020 •

edited

Loading