Skip to content

Conversation

@hd140283
Copy link
Contributor

Fixes # (no related issue)

  • npm run test succeeds.
  • npm run lint succeeds.
  • Appropriate changes to README are included in PR. (N/A)

Summary

This PR fixes file path normalization in file operations to ensure compatibility with Windows environments.

Details

  • Corrected path normalization logic to handle Windows-specific file paths.
  • Updated test cases to verify correct path normalization on Windows.

Testing

All tests and lint checks were executed successfully on a Windows environment.

@sqrrrl sqrrrl merged commit edd619f into google:master Mar 26, 2025
2 checks passed
@hd140283 hd140283 deleted the fix/windows-path-issue branch March 27, 2025 07:47
const resolvedPath = path.relative(contentDir, localPath);
const parsedPath = path.parse(resolvedPath);
let remotePath = path.format({dir: normalizePath(parsedPath.dir), name: parsedPath.name});
let remotePath = normalizePath(path.format({dir: parsedPath.dir, name: parsedPath.name}));
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the remote filepath format be dependent on the local OS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to clarify:

The remote path is normalized using normalize-path, so it's always converted to POSIX format (/), regardless of the local OS. This ensures consistent behavior across different environments.

That logic was implemented in my PR and has already been merged into the master branch — but it hasn’t been released to NPM yet.

If you're using the latest published version of @google/clasp, the change may not be included yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just moved normalizePath() to wrap the result of path.format(), since path.format() is OS-dependent.

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.

3 participants