feat(rome_cli): Add rage command
#3436
Conversation
✅ Deploy Preview for rometools canceled.
|
58d6e7e to
19fdf7d
Compare
2669d34 to
0195126
Compare
|
Comparing feat(rome_cli): Add
1 page tested
|
| Chrome Desktop | iPhone, 4G LTE | Motorola Moto G Power, 3G connection |
|---|---|---|
 |
 |
 |
Most significant changes
 |
Value | Budget |
|---|---|---|
| JS Parse & Compile Motorola Moto G Power, 3G connection |
2.18s  from 26ms |
 |
| Total JavaScript Size in Bytes iPhone, 4G LTE |
4.32 MB  from 86.8 KB |
|
| Total JavaScript Size in Bytes Chrome Desktop |
4.32 MB  from 86.8 KB |
|
| Total JavaScript Size in Bytes Motorola Moto G Power, 3G connection |
4.32 MB from 86.8 KB |
|
| JS Parse & Compile Chrome Desktop |
3.14s  from 65ms |
|
28 other significant changes: JS Parse & Compile on iPhone, 4G LTE, First Contentful Paint on Motorola Moto G Power, 3G connection, Largest Contentful Paint on Motorola Moto G Power, 3G connection, Speed Index on Motorola Moto G Power, 3G connection, First Contentful Paint on Chrome Desktop, Total Page Size in Bytes on iPhone, 4G LTE, Total Page Size in Bytes on Chrome Desktop, Total Page Size in Bytes on Motorola Moto G Power, 3G connection, Number of Requests on Chrome Desktop, Number of Requests on iPhone, 4G LTE, Number of Requests on Motorola Moto G Power, 3G connection, Time to Interactive on Motorola Moto G Power, 3G connection, Time to Interactive on Chrome Desktop, Speed Index on Chrome Desktop, First Contentful Paint on iPhone, 4G LTE, Total Blocking Time on Motorola Moto G Power, 3G connection, Largest Contentful Paint on Chrome Desktop, Speed Index on iPhone, 4G LTE, Largest Contentful Paint on iPhone, 4G LTE, Time to Interactive on iPhone, 4G LTE, Total Image Size in Bytes on Chrome Desktop, Total Image Size in Bytes on iPhone, 4G LTE, Total Image Size in Bytes on Motorola Moto G Power, 3G connection, Total HTML Size in Bytes on Chrome Desktop, Total HTML Size in Bytes on iPhone, 4G LTE, Total HTML Size in Bytes on Motorola Moto G Power, 3G connection, Lighthouse Performance Score on Motorola Moto G Power, 3G connection, Lighthouse Performance Score on Chrome Desktop
Calibre: Site dashboard | View this PR | Edit settings | View documentation
There was a problem hiding this comment.
Here's some feedback based on the snapshot/examples given:
Discovering running Rome servers..., this doesn't seem to be an useful information for us... can we omit it?✖ Workspace rage failed: Method not found. If this information is useful for us, then it should contain more details... what does that mean? If it's not useful for us, can we omit it?- What's the objective of the configuration part? At the moment it only shows if linter/formatter are disabled. Do we show all options? If so, would it be possible to see how - for example - options around rules are rendered? Or even the formatter.. just to have a look and feel of the final result.
Rome Configuration: Status: loaded Formatter disabled: false Linter disabled: false
| } | ||
| } | ||
|
|
||
| struct KeyValuePair<'a>(&'a str, Markup<'a>); |
There was a problem hiding this comment.
I think KeyValuePair could be a trait or have a new trait RageDisplay that writes into a buffer, this would allow us to print sections of the configuration as we go, here's some meta code:
trait RageDisplay {
fn display(&self, fmt: &mut rome_console::fmt::Formatter) -> io::Result<()> {}
}
impl RageDisplay for JavascriptFormatter {
fn display(&self, fmt) -> {
fmt.write_markup(buffer, markup!{ KeyValuePair("Quote style", markup!{}) })?;
fmt.write_markup(markup!{ KeyValuePair("Quote properties", markup!{}) })?;
}
}There was a problem hiding this comment.
It's intentional that we write the whole markup in one go and it is already writing into whatever Buffer console.log uses.
It would also be possible to write the output as multiple separate logs if this is what we want (all of the implementations gracefully handle errors which is why I think this isn't necessary) by simply calling console.log(markup) multiple times.
The intention here is to print some output in case the
Having more information would be useful but I can't emit more than
I don't know. I considered printing the full configuration but that may not be safe because it may contain path information. I then opted to print some basic information. One option could be to print the full configuration in the future as JSON and print "set"/"unset" for all values except for keys that are on an allowlist. |
Summary
This PR adds a
ragecommand to Rome's CLI.The
ragecommand aims to print useful information about the status of Rome and the project to help debug issues with Rome or the setup of a project.This PR implements a "basic" rage command that
This PR does not yet implement a way to retrieve the logs I'll give it a short try as a separate PR.
Architecture
I considered three different designs on how to model the result of the
rome/rageLSP command and went for the second option. I would be interested to hear your thoughts.Structured type
Make
RageResulta structured type with fields for every single value that we expect to be returnedThe upside of this approach is that it gives the client the most flexibility when it comes to layouting the rage output.
I decided against this approach because I fear that adding or removing
fieldsotherwise risks a version mismatch between client and LSP results inPartially structured
This is the approach implemented by this PR. The idea is to generally encode enough information to enable the client to nicely print the rage output but without enforcing an overly strict data layout.
RageResultis defined as:Where
Markupis an escape hatch for all cases where neitherSectionnorPairare applicable.The upside is that this should be a loose enough format where the LSP can add new fields but have enough information to layout the data on the client.
The main downside is that while this approach gives the impression that the client display and LSP logic are independent this isn't entirely true because the order in which the entries are returned does matter for a logical structured output in the CLI.
Unstructured
The third Option would be to change
RageResultto return ajsonbag, giving full flexibility to the LSP. We may be able to format such content nicely in the future when our JSON formatter is ready but it significantly increases the complexity in the CLI to show a nice rage output.Diagnostics
I didn't fully explore this option and maybe @leops may share his thoughts if it may be a good use case. The idea would be that
RageResultreturns aDiagnosticthat could even be nested inside each other (to achieve the same level of nesting as the current solution).The reason why I didn't further investigate this approach is because I don't consider
rageitself as aDiagnosticand using a purpose-built data structure yields additional flexibility.Test Plan
No running server
Running server but without `--use-server
With
--use-serverWith old LSP server
Malformed Configuration
No configuration