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

[9.x] Introducing a fresh new look for Artisan #43065

Merged
merged 102 commits into from
Jul 14, 2022

Conversation

nunomaduro
Copy link
Member

@nunomaduro nunomaduro commented Jul 5, 2022

Hi, community. 👋🏻

For the past two weeks, I've been working on this pull request that introduces a fresh new look for Artisan. Before looking at the technical details, let's see some before/after comparisons:

0

1

2

3

4

5

6

7

8

9

10

11


Technical details:

A common change you will see in this pull request is all the previous command calls to $this->info, $this->warn, etc, have been replaced by $this->components->info, etc. This was made intentionally, to ensure only framework internal commands are affected by this new output.

Now, besides the regular $this->info, $this->warn, etc methods, we also introduce - in this pull request - new components that are used across the pull request. This list of components is: Alert, BulletList, Choice, Component, Confirm, Error, Factory, Info, Line, Task, TwoColumnDetail, Warn.

As an example, the $this->task is used on the "queue:work" command to display the queue job description, the result of the queue job, and its duration.

Screenshot 2022-07-11 at 19 08 39

Now, keep in mind, that the $this->components property is marked as internal. Meaning that this pull request was not meant to introduce a public API for those components, but instead, simply to improve the output of Artisan itself.

Also, these new components share something in common: they mutate the given information. By "mutate", I mean: we ensure punctuation, relative paths instead of absolute paths, and more. This is necessary to ensure things always look pretty.

This pull request effects more than 100 commands. Yet, it does not affect the following commands: completion, help, List, serve, tinker, and commands of Laravel packages such as Horizon or Telescope. In addition, it does not affect errors command from Symfony, such as invalid arguments, etc. All of this, can be done later, so we deliver this pull request first.

Finally, would be very appreciated if the community could test this pull request in their L9 projects, as we plan to ship these changes in a minor release. To test this pull request locally, please follow this instructions:

-        "laravel/framework": "^9.19.0",
+        "laravel/framework": "dev-feat/console-ui-improvements as 9.19.0",
composer update laravel/framework -W

@nunomaduro nunomaduro self-assigned this Jul 5, 2022
@nunomaduro nunomaduro changed the title [9.x] [WIP] Improve Artisan console output [9.x] Improve Artisan console output Jul 11, 2022
@nunomaduro nunomaduro changed the title [9.x] Improve Artisan console output [9.x] Introducing a fresh new look for Artisan Jul 11, 2022
@Lloydinator
Copy link

Much cleaner. I like how it looks, and I like how you're separating feedback into Info and Error.

@martinszeltins
Copy link

Beautiful! Very impressive work, can't wait to see this land.

@denniseilander
Copy link
Contributor

First of all, I really like the new interface! Great job on this one.

The only thing I'm not sure about is the choice component.
It feels like it's harder to read compared to the old version (left) when running php artisan vendor:publish

The "Provider" and "Tag" labels are not clearly distinguishable, maybe because they don't have a highlight anymore.

Screenshot 2022-07-11 at 23 19 55

The same goes for some other commands with the choice output:
On the left the old interface, on the right the new interface.

Screenshot 2022-07-11 at 23 25 46

What do you think?

@nunomaduro
Copy link
Member Author

@denniseilander Done.

@PHLAK
Copy link
Contributor

PHLAK commented Jul 11, 2022

In the vendor:publish command screenshot pre-existing files have trailing ellipses with no following word (i.e. DONE). Instead they should show SKIPPED (maybe colored yellow) at the end of the ellipses to indicate what action was taken (i.e. they were skipped). For example.

Copying directory [...] to [...] .................... DONE
File [...] already exists ........................ SKIPPED

@hmazter
Copy link
Contributor

hmazter commented Jul 12, 2022

Great work!
I don't know if it's an intentional change, but the durations for each migration file are very sporadic with this change in the migrate command.

The old output:
Duration timings on each migration
Screenshot 2022-07-12 at 08 13 52

The new output:
Duration only appears on 1 of these 5
Screenshot 2022-07-12 at 08 14 59

@nunomaduro nunomaduro force-pushed the feat/console-ui-improvements branch from 4130333 to 4a2d074 Compare July 12, 2022 09:03
@taylorotwell taylorotwell merged commit a105ae5 into 9.x Jul 14, 2022
@taylorotwell taylorotwell deleted the feat/console-ui-improvements branch July 14, 2022 14:38
@Saifallak
Copy link

it's awesome, but for queue:work

4

i would prefer to see when the job is pushed to process not just after process completed

@shaedrich
Copy link
Contributor

@Saifallak Maybe there should be a modified progress bar 🤔

@nunomaduro
Copy link
Member Author

@Saifallak @shaedrich Not sure where you got that idea.

But, when a job starts goes into "processing" you see the time and the job name. The "DONE" on the right only appears when is actually done.

@Saifallak
Copy link

@Saifallak @shaedrich Not sure where you got that idea.

But, when a job starts goes into "processing" you see the time and the job name. The "DONE" on the right only appears when is actually done.

I got that idea from screenshots,.
You made examples for everything.
Glad to know that,..
Will try it out

@L3o-pold
Copy link

This removed the id of a job when processing 😞 a105ae5#diff-68b410043fae33c9cecda76c3cf7c54a9073dd08eca46d44cb8d4424fae8dfd8L210

@ElfSundae
Copy link
Contributor

resolve() function does not exist in Lumen, fixed in #43312

@eldair
Copy link

eldair commented Jul 20, 2022

@nunomaduro was this tested on windows? This is MS terminal with WSL

image
image

@streamingsystems
Copy link

Hi All,

I opened an issue a few minutes ago:

#43325

I am trying to hunt down why my migrations don't work like they have since I upgraded to 9.21.0. I am wondering if at a glance what I am seeing might ring a bell. When I go back to 9.20.0 it works great, when I upgrade to 9.21.0 it's broken. Maybe the syntax or something changed, but that would seem odd. Anyhow, just trying to hunt it down.

@jensolafkoch
Copy link

jensolafkoch commented Jul 24, 2022

@Wirone This is fixed. We now ensure a max of 150 chars per line:

Screenshot 2022-07-13 at 12 06 29

@nunomaduro

Wouldn't it be nice to have a config option for the character count (width)? Just in case that the default seems odd in some device/environment or the distance is too far from left to right, like for many data in the new php artisan about?

Logic could be: Make width = config value (chars), but if any line of output needs more space: extend to kind of min-content.

Or optionally (config value?) make a line break an stretch over two lines. That would also work with smaller width values.

Otherwise I totally love the new design! :-)

@omerbaflah
Copy link

Fantastic work as always @nunomaduro 👏🏻

@roberttolton
Copy link

Looks great @nunomaduro but I think something might be wrong with the terminal / window width detection, or something?

Running the latest Laravel 9.26.

Screenshot 2022-08-23 at 17 32 01

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.