Skip to content

refactor: replace substr with slice & use startsWith and endsWith#2032

Open
noritaka1166 wants to merge 6 commits intomicrosoft:mainfrom
noritaka1166:refactor-code
Open

refactor: replace substr with slice & use startsWith and endsWith#2032
noritaka1166 wants to merge 6 commits intomicrosoft:mainfrom
noritaka1166:refactor-code

Conversation

@noritaka1166
Copy link
Contributor

I did the following refactoring.

  • replace deprecated substr with slice
  • use startsWith and endsWith for better readability
  • fix lint warning

if (extension !== undefined) {
const extname = path.extname(filePath);
if (extname.length === 0 && filePath[0] === '.') {
if (extname.length === 0 && filePath.startsWith('.')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this better than the array access?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbaeumer
SonarQube, which I often use, says this, but do you prefer array access?

スクリーンショット 2025-08-17 午後10 59 10

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to keep the array access.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbaeumer
I reverted it.

if (/^CLI: /.test(result)) {
result = result.substr(5);
if (result.startsWith('CLI: ')) {
result = result.slice(5);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more. Why do you use slice instead of substring. I makes it easier to see that you are manipulating a string.

end: textDocument.positionAt(diff.originalStart + diff.originalLength)
},
newText: fixedContent.substr(diff.modifiedStart, diff.modifiedLength)
newText: fixedContent.slice(diff.modifiedStart, diff.modifiedStart + diff.modifiedLength)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants