Conversation
| }; | ||
|
|
||
| const cacheKey = descriptor.resource.toString(); | ||
| const cacheKey = descriptor.resource.toString() + descriptor.etag; |
There was a problem hiding this comment.
${descriptor.resource.toString()}${descriptor.etag}
Isn't this better ? Es6 String Literals
There was a problem hiding this comment.
@amarpathak what are you smoking?
There was a problem hiding this comment.
@amadare42 That’s right. I think we can add a symbol between these two string to separate them.
${descriptor.resource.toString()}:${descriptor.etag}
There was a problem hiding this comment.
@hwhung0111 well, I think you linked wrong person, but I want to mention that es6 string templating will call toString automatically so your line can be shortened:
${descriptor.resource}:${descriptor.etag}
There was a problem hiding this comment.
This is is not the best solution. ES6 literals are not suited for this purpose but injecting dynamic data into strings.
There was a problem hiding this comment.
@amadare42 I am sorry about that. And thanks for telling me about es6 string.
@KristofferTolboll2 If I add a colon in the string, is it better to use ES6 literals.
There was a problem hiding this comment.
@hwhung0111 ES6 template literals are most used, when there are more static content, in this case, there is only the : which is static, everything else is dynamic, therefore it will decrease readability
There was a problem hiding this comment.
@KristofferTolboll2 I get your point. But why I still found ${message}: [${err.toString()} and ${this.scope}.${key} in source code? These are similar to this issue.
KristofferTolboll2
left a comment
There was a problem hiding this comment.
This is not good practise, it makes the code less readible
KristofferTolboll2
left a comment
There was a problem hiding this comment.
Don't change it Amar, it was better before
|
Thanks for the PR. It looks good to me. We're currently stabilizing for the VS Code April release so I cannot merge this at this time but have marked it to be included in the may release |
Great thanks @mjbvz |
|
This fix should be in the first VS Code 1.35 insiders build and is scheduled for the May VS code release |
This PR fix #73021 by add etag into
cacheKey. If the image is modified, the editor will not remember the previous scale.