Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

path: refactor to reduce duplicated codes #18594

Closed
wants to merge 2 commits into from

Conversation

starkwang
Copy link
Contributor

@starkwang starkwang commented Feb 6, 2018

There are many duplicated codes in methods such as normalizeString, format, extname and basename.

This change is to reduce them.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

path

@nodejs-github-bot nodejs-github-bot added the path Issues and PRs related to the path subsystem. label Feb 6, 2018
@starkwang
Copy link
Contributor Author

There are many duplicated codes in methods such as
`normalizeString`, `format`, `extname` and `basename`.
This change is to reduce them.
var res = '';
var lastSegmentLength = 0;
var lastSlash = -1;
var dots = 0;
var code;
var slashChar = win32 ? '\\' : '/';
Copy link
Contributor

Choose a reason for hiding this comment

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

we can use const here

@@ -178,6 +120,160 @@ function _format(sep, pathObject) {
return dir + sep + base;
}

function _extname(path, win32) {
assertPath(path);
var start = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use let/const instead of var in places where in possible

@starkwang
Copy link
Contributor Author

This PR will be rebased after another PR (#18654) lands.

@BridgeAR
Copy link
Member

This needs a rebase.

@starkwang
Copy link
Contributor Author

This PR is out of date and hard to rebase. I'll open another PR.

@starkwang starkwang closed this Feb 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
path Issues and PRs related to the path subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants