Handle newly introduced deps (before dvc commit) #2202
Conversation
| @@ -1 +1,8 @@ | |||
| export const shortenForLabel = (str: string): string => str.slice(0, 7) | |||
There was a problem hiding this comment.
[F] This is where the error was happening, you cannot slice null, simply adding an optional chain seemed a little dicey so I have updated the incoming types and flushed out some issues in extension/src/experiments/columns/extract.ts. The resulting function is a complete monstrosity but backwards compatible so I can live with it. Please LMK if you have better ideas... especially for the file name.
| changes: !!value && !!branch && branch?.deps?.[path]?.value !== value, | ||
| value | ||
| const value = shortenForLabel<string | null>(hash) | ||
| if (value) { |
There was a problem hiding this comment.
[F] By not collecting null here we end up with the desired output in webview (show ...).
mattseddon
changed the title
extension/src/util/string.ts
Outdated
| export const shortenForLabel = <T extends string | null>( | ||
| strOrNull: T | ||
| ): T | string => { | ||
| if (typeof strOrNull === 'string') { |
There was a problem hiding this comment.
Is there a reason not to use a ternary operator?
return typeof strOrNull === 'string' ? strOrNull?.slice(0, 7) : strOrNull
extension/src/util/string.ts
Outdated
| strOrNull: T | ||
| ): T | string => { | ||
| if (typeof strOrNull === 'string') { | ||
| return strOrNull?.slice(0, 7) |
There was a problem hiding this comment.
Why not simply check for strOrNull to be truthy instead of checking for the type to be a string? A conditional wouldn't be necessary here.
|
Code Climate has analyzed commit ad9900b and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.7% (0.0% change). View more on Code Climate. |
1/2
main<- this <- #2203Related to #2177.
When a
depis first introduced it with be returned under the workspace record in theexp showdata like this:This previously made the extension fall over. I have changed the types/code to accommodate this situation. See comments inline for details.
Screenshot