-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Add color to displayName in project configuration. #8025
Changes from 4 commits
e3bf4a3
eefc4ff
e0f09e2
ec8b7c9
7bb02fc
a810c31
789cb8b
0903cd5
e369ca5
01d8d97
94caad6
dc7b046
f75bb90
2a56599
d42b2a3
81fc522
65aa738
041a81d
01fd4aa
44429c8
ec0d0d0
68a6ae4
d1fe278
87dc8cf
f1dbd72
aada1fb
bea007a
8928a2e
c1e5a38
4462019
21b99bb
3665d38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -130,6 +130,7 @@ export type InitialOptions = { | |
detectLeaks?: boolean; | ||
detectOpenHandles?: boolean; | ||
displayName?: string; | ||
displayNameColor?: DisplayNameColor; | ||
expand?: boolean; | ||
extraGlobals?: Array<string>; | ||
filter?: Path; | ||
|
@@ -223,6 +224,25 @@ type NotifyMode = | |
| 'success-change' | ||
| 'failure-change'; | ||
|
||
type DisplayNameColor = | ||
| 'black' | ||
| 'blue ' | ||
| 'blueBright' | ||
| 'cyan' | ||
| 'cyanBright' | ||
| 'gray ' | ||
| 'green' | ||
| 'greenBright' | ||
| 'magenta' | ||
| 'magentaBright' | ||
| 'red' | ||
| 'redBright' | ||
| 'white' | ||
| 'whiteBright' | ||
| 'yellow' | ||
| 'yellowBright' | ||
|
||
|
||
export type GlobalConfig = { | ||
bail: number; | ||
changedSince: string; | ||
|
@@ -313,7 +333,8 @@ export type ProjectConfig = { | |
dependencyExtractor?: string; | ||
detectLeaks: boolean; | ||
detectOpenHandles: boolean; | ||
displayName: string | null | undefined; | ||
displayName?: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I like this implementation better actually. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the API that @SimenB is suggesting. |
||
displayNameColor?: DisplayNameColor; | ||
errorOnDeprecated: boolean; | ||
extraGlobals: Array<keyof NodeJS.Global>; | ||
filter: Path | null | undefined; | ||
|
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.
Would be good to extract these from
chalk
instead of hard coding themThere 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.
Oh I was hard coding this because of what I mentioned about not being sure if we should curate the colors that we should allow or if we should allow everything. Hence the early PR 😄
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've taken a look at the types provided by chalk and they don't expose the colors. https://github.com/chalk/chalk/blob/master/index.d.ts#L231
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.
Just as an FYI, opened up this PR. Hopefully this gets merged and we won't have to hardcode colors here. chalk/chalk#336