Conversation
Add progress messages to LGTM download option. (github#960)
| const codes: { [key: string]: any } = { | ||
| '\n': 'U+000A', | ||
| '\b': 'U+2084', | ||
| '\0': 'U+0000' | ||
| }; |
There was a problem hiding this comment.
Can you move this out of the function to the module level scope? This way, the constant doesn't need to be recreated on each invocation.
There was a problem hiding this comment.
And presumably, you will be able to add more replacements in the future?
There was a problem hiding this comment.
You can explicitly add all of the control char mappings described here #584 (comment).
| || typeof v === 'boolean' | ||
| ) { | ||
| return <span>{v.toString()}</span>; | ||
| const text = v.toString(); |
There was a problem hiding this comment.
Not a big fan of one char variable names. Can you rename it to something like rawValue. I know this code was already, but it's never too late to make improvements. :)
| for (const char in codes) { | ||
| if (char === newVal[i]) { | ||
| newVal[i] = codes[char]; | ||
| } | ||
| } |
There was a problem hiding this comment.
There's something simpler you can do here. You don't need to loop.
| for (const char in codes) { | |
| if (char === newVal[i]) { | |
| newVal[i] = codes[char]; | |
| } | |
| } | |
| newVal[i] = codes[newVal[i]] || newVal[i]; |
aeisenberg
left a comment
There was a problem hiding this comment.
Small suggestion about the change log and then 🚢 .
Co-authored-by: Andrew Eisenberg <aeisenberg@github.com>
Resolves #584
Checklist
@github/docs-content-codeqlhas been cc'd in all issues for UI or other user-facing changes made by this pull request.