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

[5.3] Replace only the first instance of the app namespace in Generators #15575

Merged
merged 1 commit into from
Sep 23, 2016
Merged

[5.3] Replace only the first instance of the app namespace in Generators #15575

merged 1 commit into from
Sep 23, 2016

Conversation

themsaid
Copy link
Member

Currently doing this php artisan make:controller App\Http\Controllers\App\AppController creates the controller in app\Http\Controllers\AppController, this PR fixes this by replacing only the first instance of the app namespace instead of all instances.

@srmklive
Copy link
Contributor

@themsaid wouldn't it be better to have a --namespace & --path options for make:controller?

@themsaid
Copy link
Member Author

@srmklive it currently guesses the path using the namespace, which is fine imho.

@maki10
Copy link
Contributor

maki10 commented Sep 23, 2016

@themsaid this not working php artisan make:controller App\AppController but working with full path.
A made I change for myself like yo do for this PR

@taylorotwell
Copy link
Member

@maki10 so are you saying there is still a problem with this PR or not?

@themsaid
Copy link
Member Author

@maki10 this should never work, you either provide the complete namespace or don't provide a namespace at all and the controller will be created in App\Http\Controllers by default.

@maki10
Copy link
Contributor

maki10 commented Sep 23, 2016

@taylorotwell Yup with php artisan make:controller App\AppController still not working.
etc. php artisan make:controller Account\AccountController working like charm. An order to create App folder and place my Controller inside I must call full path with this PR like so php artisan make:controller App\Http\Controllers\App\AppController

@themsaid
Copy link
Member Author

themsaid commented Sep 23, 2016

php artisan make:controller Account\AccountController creates a controller in Http\Controllers still

@maki10
Copy link
Contributor

maki10 commented Sep 23, 2016

@themsaid Yes, i know that but when you want to place your controller to the folder and in my case folder is App than to what? Copy + paste?
@themsaid Yup inside folder Account

@themsaid
Copy link
Member Author

themsaid commented Sep 23, 2016

@maki10 no not inside a folder, it'll place it in app\Http\Controllers, you have two options:

  1. Provide a full namespace
  2. Don't provide a namespace at all and the Controller will be create in app\Http\Controllers

@maki10
Copy link
Contributor

maki10 commented Sep 23, 2016

@themsaid What is that 'place' if is not folder??? php artisan make:controller make controller under namespace App\Http\Controllers\* right?
If I provide Account\AccountController it while be created folder Account if not exist and placed controller inside with namespace App\Http\Controllers\Account.
For option 1, this PR is working like a charm.

@themsaid
Copy link
Member Author

@maki10 hold on, I'm making edits that may solve your issue.

@themsaid
Copy link
Member Author

@maki10 Taking a deeper look at the codebase, there's no way this can be fixed, the thing is, the root namespace is reserved as it's used to check if you're trying to create using a full path or not.

If your class starts with App\\ Laravel assumes you'll be giving a full namespace, if not then it'll assume you're using the default namespace for every component.

To be able to use the root namespace inside a default path you'll have to provide the full path, e.g.:

php artisan make:controller App\\Http\\Controllers\\App\\AppController

But that isn't possible because Laravel would replace all App\\ in the namespace leaving you with Http\\Controllers\\AppController which explains the behaviour you reported in the issue.

This PR fixes the replace all issue by only replacing the first instance of App, allowing you to do this:

php artisan make:controller App\\Http\\Controllers\\App\\AppController

But doing make:controller App\\AppController will make laravel assume you want to create it in the App root namespace, and changing this behaviour might break other generation commands.

So my conclusion is that you need to consider the app namespace as a reserved namespace, if you want to use it then you have to provide a full path and never rely on the default path.

@maki10
Copy link
Contributor

maki10 commented Sep 23, 2016

@themsaid nice to know that, so solution for myself is to provide a full path in order to place my controller in App folder inside Controllers. With this PR is possible to place controller in desire path with full namespace.

So +1 for PR

@taylorotwell taylorotwell merged commit 574848f into laravel:5.3 Sep 23, 2016
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

4 participants