feat: colorize values of params #743
Conversation
|
Looks very promising! I'll take a look at the code tomorrow. |
|
@StefanDywersant, I've addressed the feedback. Colorizing is now optional as |
There was a problem hiding this comment.
I've dropped some comments but in general looks good.
I wish we could be a little bit smarter and try to modify the parameters as we construct the URLs, but unfortunately no URI Template library will allow us to do that. I guess we will have to go with what we have.
lag-of-death
requested review from
karol-maciaszek and
XVincentX
and removed request for
karol-maciaszek
packages/cli/src/util/colorizer.ts
Outdated
| transform: (aString: string) => string | ||
| ): string => { | ||
| return path.replace(taggedParamsValues, transform('$2')); | ||
| }).bind({}, new RegExp(`(${PRE_PARAM_VALUE_TAG},)(.*?)(,${POST_PARAM_VALUE_TAG})`, 'gm')); |
There was a problem hiding this comment.
Hmm maybe instead of binding you can just create the RegEx ahead of the time as a constant and use it in there — this should improve the readability.
There was a problem hiding this comment.
This is a super nice feature which will make people happier, good job!
I believe there might be a problem with complex styles and explode usage. Please see the comment. Also, it would be good to add a single end2end test at create server level to ensure that the whole solution works.
Co-Authored-By: Vincenzo Chianese <vincenz.chianese@icloud.com>
…into feature/colorize-params-values_
There was a problem hiding this comment.
I've tested locally the colorizing feature and it is cool, helpful and works perfectly!
I've added a few minor comments which, most of them you can just apply and let's merge this!
Again, a very good job @lag-of-death!
Closes #696
Checklist
What kind of change does this PR introduce?
feature
What is the current behavior? What is the new behavior?
Currently, values for params are in the same color the whole path is.
Now, the values are in bold and colored.
Does this PR introduce a breaking change?
No.