Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Console][FrameworkBundle] Revised console header formatting#19275

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
ro0NL wants to merge5 commits intosymfony:masterfromro0NL:frameworkbundle/console-header
Closed

[Console][FrameworkBundle] Revised console header formatting#19275

ro0NL wants to merge5 commits intosymfony:masterfromro0NL:frameworkbundle/console-header

Conversation

@ro0NL
Copy link
Contributor

@ro0NLro0NL commentedJul 3, 2016
edited
Loading

QA
Branch?"master"
Bug fix?no
New feature?yes (ui change)
BC breaks?no
Deprecations?no
Tests pass?not yet
Fixed tickets#19095
LicenseMIT
Doc PRreference to the documentation PR, if any

Before/After
image

@linaori
Copy link
Contributor

app is not really the kernel, just the name. In this case I'd expect to seeAppKernel, would that be nicer to show?

@ro0NL
Copy link
ContributorAuthor

ro0NL commentedJul 3, 2016
edited
Loading

If it's namespaced it can get messy.. but you're right; it's different information. Both are useful :')

What about;
Symfony 3.2.0-DEV on \My\AppKernel(name: app, env: dev, debug: true)

Or with verbosity;

$ bin/console -VSymfony 3.2.0-DEV

vs.

$ bin/console -V --verboseSymfony 3.2.0-DEV on \My\AppKernel(name: app, env: dev, debug: true)

But im not sure thats common or even semantically correct..

@linaori
Copy link
Contributor

I personally prefer the verbosity flag but I don't know how intuitive this is.

@javiereguiluz
Copy link
Member

Another solution would be to remove thename: app thing and to not add the kernel info either.

@linaori
Copy link
Contributor

There might be a lot more info about the kernel you'd want to see... What about simply adding the version + release date (if available) and making an application info command? Not sure if this would be interesting for the core, but it might show nice things such as which bundles are enabled.

javiereguiluz reacted with thumbs down emoji

@ro0NL
Copy link
ContributorAuthor

@iltar i was just thinking about that.. with--verbose we can go really crazy;

image

javiereguiluz reacted with thumbs down emoji

@linaori
Copy link
Contributor

I'm afraid that eventually it will come down to this: We need a command line WDT variant

@ro0NL
Copy link
ContributorAuthor

Agree, this is still not great as a header line when used withbin/console. So what do we need to show for-V then? (assuming everything is available withbin/console info in some time).

Perhaps kernel info is a bit too specific indeed =/

@linaori
Copy link
Contributor

For now I'd say a limit of what's available in theKernel constants. I would love to have more debugging info on the command line though, such as a CLI WDT. Maybe a nice feature for 3.2 (though I have no time to work on this).

@ro0NL
Copy link
ContributorAuthor

ro0NL commentedJul 3, 2016
edited
Loading

removed kernel:info command

@javiereguiluz
Copy link
Member

Please, let's focus first on finishing this pull request about the console header. Then we can discuss about thekernel:info command in a separate issue. Thanks!

@ro0NL
Copy link
ContributorAuthor

Done.

.sprintf(
' (env: <comment>%s</comment>, debug: <comment>%s</comment>)',
$this->kernel->getEnvironment(),$this->kernel->isDebug() ?'true' :'false'
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This should be on one line.

Copy link
ContributorAuthor

@ro0NLro0NLJul 4, 2016
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Can i go with<comment>%s</> (vs.<comment>%s</comment>) btw? Would improve readability in a way :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

IIRC we already do that in other places.

@ro0NLro0NL changed the title[WIP] Revised console header formatting[Console][FrameworkBundle] Revised console header formattingJul 4, 2016
@fabpot
Copy link
Member

I would not remove the app name here. You can have several one and being able to have it with-V is important.

@fabpot
Copy link
Member

Any news on this one?

@fabpot
Copy link
Member

@ro0NL Still interested in finishing this one?

@ro0NL
Copy link
ContributorAuthor

Yes. Try to work on it today. As soon as my laptop is connected :(

Op 19 jul. 2016 17:35 schreef "Fabien Potencier"notifications@github.com:

@ro0NLhttps://github.com/ro0NL Still interested in finishing this one?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#19275 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA_8kAIy7NN_USpVFhfpth5vuvP7fo8Pks5qXO68gaJpZM4JD0O5
.

@ro0NL
Copy link
ContributorAuthor

ro0NL commentedJul 20, 2016
edited
Loading

Back up :)@fabpot it's going to beSymfony 3.2.0-DEV (kernel: app, env: dev, debug: true) then.. wherekernel refers the kernel name, not the kernel type.

I think it makes sense.. if#19278 is accepted we have more detailed information anyway.

Maybe the minor UI change for console applications is worth noting..., not sure.

@fabpot
Copy link
Member

Tests should be updated as they do not pass with these changes.

@ro0NL
Copy link
ContributorAuthor

Green :)

@fabpot
Copy link
Member

Thank you@ro0NL.

@ro0NLro0NL deleted the frameworkbundle/console-header branchAugust 6, 2016 07:35
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ro0NL@linaori@javiereguiluz@fabpot@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp