-
-
Notifications
You must be signed in to change notification settings - Fork 125
Add --open flag to server:start command
#668
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
Conversation
--open flag to server:start command
|
I'm not sure I'd like to always open a browser. |
|
I think it depends on what kind of project you're working. Indeed that's not useful for an API-oriented application, but it is for a Twig-based app. I've worked on quite a few JavaScript projects (mostly based on Vite), and having the app open automatically in the browser was super convenient. Here, the option is disabled by default, and it helps to resolves #639 (where you have to scroll a tons of log to find the application URL). I think that's fine? |
| func abstractOpenCmd(url string) { | ||
| if err := open.Run(url); err != nil { | ||
| terminal.Eprintln("<error>Error while opening:", err, "</>") | ||
| terminal.Eprintfln("Please visit <href=%s>%s</> manually.", url, url) | ||
| } else { | ||
| terminal.Eprintfln("Opened: <href=%s>%s</>", url, url) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening an URL does not really makes sense outside of an interactive context (ie when running a command). So I would not move it to utils.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I wasn't really sure, I just wanted to centralize this logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure where to put it, so I simply reverted my changes and duplicated the block to open the URL in the browser
|
I think I agree with @fabpot here: while I see the use of the |
|
A setting
Why can't we add it to the Symfony CLI? |
|
I'm not saying we can't, I'm saying I'm not sure this is that useful (again in the configuration as I'm convinced of the flag. Because from experience I usually do something like But I'm not against having it |
|
I'm fine adding the |
ee0faee to
88da774
Compare
|
I just removed the |
|
It is if you are the same package (ie. directory)
… Message ID: ***@***.***
com>
|
88da774 to
c2ead21
Compare
local/project/config.go
Outdated
|
|
||
| NoWorkers bool | ||
| Daemon bool | ||
| Open bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be part of the config. Instead, you should create a flag in the server:start command directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, there's no need for that anymore.
Fixed!
c2ead21 to
7cbdcd8
Compare
7cbdcd8 to
1522ce6
Compare
|
Thank you @Kocal. |
Related to #639
Passing the flag
--opentosymfony serveorsymfony server:startopen the first reachable URL in the default web-browser.