-
Notifications
You must be signed in to change notification settings - Fork 69
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: slimmer ux #1056
feat: slimmer ux #1056
Conversation
src/config/config.ts
Outdated
this.theme = theme | ||
this.theme = await this.loadTheme() | ||
|
||
const s3 = this.pjson.oclif.update?.s3 ?? { |
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 don't like this style (declaring a const object with placeholders).
could there be a function whose single responsibility is to build the s3
object, or the updateConfig
object?
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.
and if the property is optional, we could type it that way rather than using empty string
@@ -97,11 +97,13 @@ export class Config implements IConfig { | |||
public shell!: string | |||
public theme?: Theme | |||
public topicSeparator: ' ' | ':' = ':' | |||
public updateConfig!: NonNullable<PJSON.CLI['oclif']['update']> |
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.
if we're doing breaking changes, maybe we could make the types honest? Like, these !
are kinda true as long as you've called the static load
method?
And TS can't help developers avoid runtime undefineds if we do stuff like that.
noodle on it
src/errors/warn.ts
Outdated
/** | ||
* Prints a pretty warning message to stderr. | ||
*/ | ||
export function warn(input: Error | string, options?: {ignoreDuplicates: boolean}): void { |
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.
maybe jsdoc the ignoreDuplicates option?
return stripAnsi(this.formatIfCommand(example)).startsWith( | ||
`${colorize(this.config?.theme?.dollarSign, '$')} ${this.config.bin}`, | ||
) | ||
return ansis |
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.
ooh, that's nice
src/ux/theme.ts
Outdated
.map(([key, value]) => { | ||
if (typeof value === 'string') return [key, isValid(value)] | ||
return [key, parseTheme(value)] | ||
}) |
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.
.map(([key, value]) => { | |
if (typeof value === 'string') return [key, isValid(value)] | |
return [key, parseTheme(value)] | |
}) | |
.map(([key, value]) => [key, typeof value === 'string' ? isValid(value) : parseTheme(value)]}) |
* chore: bump major version * feat: slimmer ux (#1056) * chore: bump major version * feat: slimmer ux module * test: no fancy-tests * chore(release): 4.0.0-v4.0 [skip ci] * chore: code review * chore(release): 4.0.0-v4.1 [skip ci] * chore(release): 4.0.0-beta.1 [skip ci] * chore: remove duplicate dep * test: windows unit tests * test: windows unit tests --------- Co-authored-by: svc-cli-bot <[email protected]> * chore(release): 4.0.0-beta.2 [skip ci] * feat: add customizable logger * feat: remove native error logger * fix: cache child loggers * chore(release): 4.0.0-beta.3 [skip ci] * chore(release): 4.0.0-beta.4 [skip ci] * feat: support rc files (#1067) * feat: support rc files * chore: code review * chore(release): 4.0.0-beta.5 [skip ci] * chore(release): 4.0.0-beta.6 [skip ci] * fix: revert ignoreDuplicates in warn * chore(release): 4.0.0-beta.7 [skip ci] * feat: improved types and top-level exports (#1076) * feat: improved PJSON type * feat: enable exactOptionalPropertyTypes * feat: theme can be object or file path * feat: support target and identifier for helpClass * chore: add jsdocs to Configuration type * fix: add back scope to pjson type * feat: top level exports * fix: simplify ux export * chore: cleanup * feat: export logger too * chore(release): 4.0.0-beta.8 [skip ci] * feat: remove baseFlags * test: use oclif/test v4 and improve test coverage * fix: clarify types * test: use default sinon sandbox * test: use new major of oclif/test * test: add interop test for core v3 * test: skip spinner test on windows * chore: code review * chore(release): 4.0.0-beta.9 [skip ci] * fix: restore baseFlags support (#1085) * chore(release): 4.0.0-beta.10 [skip ci] * fix: improve types and ProdOnlyCache * chore(release): 4.0.0-beta.11 [skip ci] * fix: update hook type * chore(release): 4.0.0-beta.12 [skip ci] * fix: allow empty ux.stdout * chore(release): 4.0.0-beta.13 [skip ci] * chore: add ux README * fix: check supports-color in colorize * chore(release): 4.0.0-beta.14 [skip ci] * feat: support tsx for runtime transpilation * chore(release): 4.0.0-beta.15 [skip ci] * chore(release): 4.0.0-beta.16 [skip ci] --------- Co-authored-by: svc-cli-bot <[email protected]> Co-authored-by: Steve Hetzel <[email protected]> Co-authored-by: Willie Ruemmele <[email protected]>
Slimmer
ux
As described here, we're removing most of the methods in the
ux
module. We're simply unable to adequately support the feature set thatux
offers and think that most people would benefit from using dedicated libraries that are better supported.We are, however, keeping some of the functionality. The new
ux
module will contain the following:Unchanged
colorize
error
exit
action
- will be unchanged from previous version except that the spinner color will be configurable using themes.warn
Renamed
stdout
- rename ofux.log
stderr
- rename ofux.logToStderr
New
colorizeJson
- Apply color theme to arbitrary JSON.What you'll need to do
You will need to replace everything that
ux
was doing with dedicated libraries. Here are a few suggestions:No fancy-tests
@oclif/test
andfancy-test
test/test.ts
, which will soon become the next major of@oclif/test