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

Generate file names with types by writing a dot instead of an extra spaced type parameter #2155

Closed
shairez opened this issue Sep 15, 2016 · 10 comments · Fixed by #2707
Closed
Assignees

Comments

@shairez
Copy link
Contributor

shairez commented Sep 15, 2016

I've tried to generate a new model class by running ng generate class hero/here.model

But it generated - hero/hero.ts and ignored what was after the dot.

@Brocco told me that it's because we should pass the "type" parameter with a space and not with a dot, like this - ng generate class hero/here model

I think that writing the file name as I want it to be (with the dot) is much more intuitive and not making developers go and read the docs to figure out what happened to their precious type :)
With not ignoring the "dot type" for the filename (but do ignoring it for the Class name), it'll just work more intuitively IMO.

Mike suggested to maybe we should have both options, after thinking about it I think it's always better to have just one choice rather than two, to minimize confusion and redundant documentation.

@hansl , @filipesilva , @Brocco I'm interested to hear what you think?

Does this make sense? if so, should we keep them both (dot and space) or just one?

Once we'll figure it out, I'll create the PR

Thanks!

@filipesilva
Copy link
Contributor

I think it's more intuitive to just do ng generate class hero.model. But I also remember that last time we talked about this, there was a fairly technical reason why we didn't do that.

@Brocco
Copy link
Contributor

Brocco commented Sep 16, 2016

I prefer to leave it with the space, but understand the case for the . notation.

The reason I prefer the space is because you are defining two different things.

  • The name of the class
  • The type of class

They are separate because the name of the class is used in both the class name and the file name where the type is **only* part of the file name. I just feel like the space is a clearer separation when generating a class.

@shairez
Copy link
Contributor Author

shairez commented Sep 16, 2016

@Brocco I see your point, and it's a valid one.
I'm just not sure it is justifies the extra step of figuring this distinction out in the docs or the --help.

As I wrote, I think the dot notation will be the first intuitive thing newcomers will try, because this is how you usually create regular files outside of the cli, also how you are use to import them (leaving the extension out)

@hansl @Brocco was there a technical reason it isn't possible like @filipesilva remembers?

@filipesilva
Copy link
Contributor

That was it actually - the class name, within the file. We can't discern if 'hero-name.villain.model' should be class HeroNameVillain or HeroNameVillainModel.

@shairez
Copy link
Contributor Author

shairez commented Sep 16, 2016

But currently, it is being ignored anyway (as I stated in my first comment)

I think it is reasonable to ignore what after the dot for classes, as they are too general and not part of the Angular world.

@Brocco
Copy link
Contributor

Brocco commented Sep 16, 2016

I'll fall back to my original statement... I'm good with the idea of adding the . parsing of the first argument, but leaving the current functionality. Which just leaves the scenario of how to handle if someone were to do this:
ng g class foo.bar buzz
What takes precedence? bar or buzz? or would that throw an error?

@filipesilva
Copy link
Contributor

Doing ng g class foo.bar buzz I would expect to have foo.bar.buzz.ts generated, with class Foo inside.

Maybe that's the way to go, class name is up to first dot, filename is everything.

@Brocco
Copy link
Contributor

Brocco commented Sep 22, 2016

I can concede to this.

Brocco added a commit to Brocco/angular-cli that referenced this issue Oct 13, 2016
Closes angular#2155

BREAKING CHANGE: The ability to specify a class type via an additional arg has been replaced by combining the name and type args separated by a dot
Brocco added a commit to Brocco/angular-cli that referenced this issue Oct 14, 2016
Closes angular#2155

BREAKING CHANGE: The ability to specify a class type via an additional arg has been replaced by combining the name and type args separated by a dot
filipesilva pushed a commit that referenced this issue Oct 14, 2016
Closes #2155

BREAKING CHANGE: The ability to specify a class type via an additional arg has been replaced by combining the name and type args separated by a dot
@shairez
Copy link
Contributor Author

shairez commented Oct 14, 2016

Thanks @Brocco !

Brocco added a commit that referenced this issue Oct 19, 2016
Closes #2155

BREAKING CHANGE: The ability to specify a class type via an additional arg has been replaced by combining the name and type args separated by a dot
texel pushed a commit to splice/angular-cli that referenced this issue Nov 3, 2016
Closes angular#2155

BREAKING CHANGE: The ability to specify a class type via an additional arg has been replaced by combining the name and type args separated by a dot
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants