feat: slimmer ux #1056
Merged
feat: slimmer ux #1056
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mshanemc
reviewed
Apr 17, 2024
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.
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.
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.
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.
maybe jsdoc the ignoreDuplicates option?
| return stripAnsi(this.formatIfCommand(example)).startsWith( | ||
| `${colorize(this.config?.theme?.dollarSign, '$')} ${this.config.bin}`, | ||
| ) | ||
| return ansis |
src/ux/theme.ts
Outdated
Comment on lines
33
to
36
| .map(([key, value]) => { | ||
| if (typeof value === 'string') return [key, isValid(value)] | ||
| return [key, parseTheme(value)] | ||
| }) |
There was a problem hiding this comment.
Suggested change
| .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)]}) |
mshanemc
approved these changes
Apr 22, 2024
mdonnalley
added a commit
that referenced
this pull request
Jun 4, 2024
* 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 <Svc_cli_bot@salesforce.com> * 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 <Svc_cli_bot@salesforce.com> Co-authored-by: Steve Hetzel <shetzel@salesforce.com> Co-authored-by: Willie Ruemmele <willieruemmele@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Slimmer
uxAs described here, we're removing most of the methods in the
uxmodule. We're simply unable to adequately support the feature set thatuxoffers 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
uxmodule will contain the following:Unchanged
colorizeerrorexitaction- will be unchanged from previous version except that the spinner color will be configurable using themes.warnRenamed
stdout- rename ofux.logstderr- rename ofux.logToStderrNew
colorizeJson- Apply color theme to arbitrary JSON.What you'll need to do
You will need to replace everything that
uxwas doing with dedicated libraries. Here are a few suggestions:No fancy-tests
@oclif/testandfancy-testtest/test.ts, which will soon become the next major of@oclif/test