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

[multisite:new] argument 'uri' is un-used. #3508

Closed
mattsqd opened this issue Aug 31, 2017 · 4 comments
Closed

[multisite:new] argument 'uri' is un-used. #3508

mattsqd opened this issue Aug 31, 2017 · 4 comments

Comments

@mattsqd
Copy link

mattsqd commented Aug 31, 2017

[ command:name] multisite:new argument 'uri' is un-used.

Problem/Motivation

multisite:new documentation
When passing the second argument to the second parameter, it is unused by src/Command/Multisite/NewCommand.php. What happens is that the first argument, the sites directory, is used for both key and value of sites.php.

Details to include:

  • I believe a change was made here, that I believe was because the confusing term 'uri' is used in this command that is different than other commands.
  • URI sometimes means the URL to reach the site https://test.com, the URI as in test.com, but in this case, shouldn't it be keys like seen in this example?

How to reproduce

drupal multisite:new my_site_dir my_site_uri
When finished, sites.php should contain an entry with my_site_dir as key and value.

Details to include:

  • Drupal version 8.3.7
  • Console version 1.0.1.
  • Console Launcher version 1.0.1.

Solution

Revert this commit

Update the documentation to reference how keys of sites.php are created.

@jmolivas jmolivas changed the title [ command:name] multisite:new argument 'uri' is un-used. [multisite:new] argument 'uri' is un-used. Sep 1, 2017
@cburschka-pwc
Copy link
Contributor

cburschka-pwc commented Jan 15, 2018

I'm struggling to understand the purpose of that commit - mapping the directory name to itself is obviously not correct, and I don't see how it fixes anything it was supposed to fix.

However, the original code was also wrong, and it doesn't seem trivial. Just as you say: The correct syntax of the array key in $sites is not a URI, but a dotted string mask, with various possible values.

I suspect the best fix would be reverting the bad commit, and renaming the "URI" to a more accurate "mask" argument: drupal multisite:new <directory> <mask>

As a convenient feature, if no mask is given, the command could interactively list suggestions based on the value of a --uri option.

Eg: drupal multisite:new mysite www.mysite.com does exactly that, while drupal multisite:new mysite --uri https://www.mysite.com:8888 does something like the following:

$ drupal multisite:new mysite --uri https://www.mysite.com:8888/test/

Choose one of the following masks:

[0] 8888.www.mysite.com.test
[1] 8888.www.mysite.com
[2] www.mysite.com.test
[3] www.mysite.com
[4] mysite.com

(Note that there are very many possible values here, including some that are very unlikely like com for http://*.com/, so it would make sense to only list a few of the most common ones and let the user otherwise pass one directly via the mask argument.)

@rpsu
Copy link
Contributor

rpsu commented May 17, 2018

PR #3232 solves one problem leaving the functionality half broken.

Currently there is no way to create aliases to sites.php, generated alias serves only as a template for the correct alias to be written.

I suppose due to why it was originally done (#3197) it can't be simply overwritten, but maybe creating 2 aliases could solve this? Around here we would create 2 aliases instead of the one that apparently breaks chainability.
$sites_file_contents .= "\n\$sites['$uri'] = '$this->directory';";
$sites_file_contents .= "\n\$sites['$this->directory'] = '$this->directory';";

@CaptainQuirk
Copy link

Hi there,

I'm a bit baffled by the PR #3232 and the reason it was merged into the code base. It seems to me that the doc is faulty there.

It gives the impression that you should use a URL (http://mysite.example.com) although is a URI that should be used, just like the one you use when adding the --uri flag to various drupal-console or drush commands.

It also gives the impression that a new site should reside in the vendor directory, which is kind of misleading too.

Would it be possible, without creating a service as elaborate as @cburschka-pwc proposes, to just revert the PR and let the end user call the command with whatever he likes as a uri. Correct me if I'm wrong, but there is a whole comment section in the sites.php file explaining all the patterns that can be used to make Drupal map a url to a directory within the sites directory.

That being said, having a mask parameter would be great to.

@hjuarez20
Copy link
Contributor

Hi, @mattsqd We add the uri as argument in this PR #3871 that is available on DC 1.9.0

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

No branches or pull requests

6 participants