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

feat(pretty-print): add a helper to print relative paths #146

Merged
merged 2 commits into from
Dec 7, 2021
Merged

Conversation

varl
Copy link
Contributor

@varl varl commented Nov 18, 2021

feat(pretty-print): add a helper to print relative paths

This introduces a "pretty print" module that we can use to introduce
helpers for pretty print strings.

Printing relative file paths is something that comes up a lot. We want
to use the absolute path in the code as that makes everything more
robust when the cwd is manipulated or different from the expectation.

In those cases, it makes sense to have a function that just transforms
the absolute path to a relative path:

    pp.relativePath('/home/foo/bar', '/home')
    // output: foo/bar

The second argument defaults to process.cwd() and should be omitted for
normal use. We are interested in the user's current working directory
after all.

In cases where the user's cwd does not match what we want to show as
output, we can set the cwd to another path, such as the root project
directory to print src references relative to that.
@varl varl changed the title pretty print feat(pretty-print): add a helper to print relative paths Nov 18, 2021
Copy link
Member

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

Why the extra module here? I.e. do we anticipate other prettyPrint utilities?

Current usage appears to be:

const { prettyPrint } = require('@dhis2/cli-helpers-engine')

prettyPrint.relativePath('C:\Program Files (x86)\Steam\steamapps\common\Doom 3')

That's maybe a little verbose? I do like protecting the top-level namespace though.

@varl
Copy link
Contributor Author

varl commented Nov 30, 2021

When I wrote this I had a few other prettyPrint functions in mind, but time is an uncaring construct and it has taken them from me.

There are some patterns I'd like to be more consistent with throughout all our CLI modules.

E.g. when a command in d2-style tells the user what it's doing it uses the command > subcommand as a header before deferring to the output from the process:

javascript > eslint
All matched files pass the lint rules.

javascript > prettier
Checking formatting...
All matched files use Prettier code style!

structured-text > prettier
Checking formatting...
All matched files use Prettier code style!

d2-app-scripts doesn't use headers and instead prints information flat:

Build parameters:
 - Mode : production
 - PUBLIC_URL : .
 - DHIS2_BASE_URL : ../../..
Generating internationalization strings...
Writing 2 language strings to ./i18n/en.pot...
Bootstrapping local appShell...
Copying appShell to temporary directory...
Building app data-entry...
Generating manifests...
Building appShell...
Creating archive from build/app at build/bundle/data-entry-1.0.0.zip...
Total size: 1670407 bytes

**** DONE! ****

d2-cluster is more like d2-style in that it prints a header, and then defers the output to the process it started:

Spinning up cluster latest
Network d2-cluster-latest_default  Creating

Network d2-cluster-latest_default  Created

Container d2-cluster-latest-gateway-1  Creating

Container d2-cluster-latest-db-1  Creating

Container d2-cluster-latest-db-1  Created

Container d2-cluster-latest-core-1  Creating

Container d2-cluster-latest-gateway-1  Created

Container d2-cluster-latest-core-1  Created

Container d2-cluster-latest-db-1  Starting

Container d2-cluster-latest-gateway-1  Starting

Container d2-cluster-latest-gateway-1  Started

Container d2-cluster-latest-db-1  Started

Container d2-cluster-latest-core-1  Starting

Container d2-cluster-latest-core-1  Started

So my thinking was that the prettyPrint module could provide functions that make it easier to be consistent across multiple modules for stuff.

E.g.

prettyPrint.header('javascript')
// output: > javascript

prettyPrint.header('javascript', 'eslint')
// output: > javascript > eslint

prettyPrint.header('javascript', 'eslint', 'staged')
// output: > javascript > eslint > staged

And if we want to render it differently it can be changed in one place instead of each module.

Copy link
Member

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

Got it, sounds great to me - let's keep the prettyPrint module!

@varl varl merged commit ed56b2b into master Dec 7, 2021
@varl varl deleted the pretty-print branch December 7, 2021 11:16
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 3.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants