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

refactor(ls): Enhance compatibility and optimize path handling for Windows system #9

Merged
merged 11 commits into from
May 21, 2024

Conversation

mitsuki31
Copy link
Owner

Overview

This pull request introduces several enhancements and fixes aimed at improving support for Windows systems. Notable changes include additions of new functions for resolving file URLs and detecting Windows paths, along with fixes for path resolution issues and improvements in error handling. Additionally, there are updates to scripts and deprecation of internal function for better code organization.

Notable Changes

Refactors and Improvements

  • Added new npm scripts for project building and testing (see 62bce18 to check all added scripts).
  • Added a new function to resolve file URL paths, along with regex and function for path detection.
  • Enhanced Windows path resolution in the ls function to handle drive letter paths.
  • Addressed errors while stating read-protected entries and improved directory entry handling.
  • Re-sorted imports in lsfnd module for better code maintenance.

Deprecation

  • Deprecated an internal function (fileUrlToPath). Because it doesn't follows the URL Standard specification and causing an issue to ls function, especially in Windows systems. It might be replaced by new function called resolveFileURL which better on resolving the file URL path (but it only accepts a string path).

    resolveFileURL definition:

    function resolveFileURL(p: StringPath): StringPath;

Test Environments

  • Fixed issues with POSIX and file URL path resolution.
  • Refactored the TestError class constructor and error handling in tests.

Summary

These changes focus on resolving an issue and enhancing stability and functionality for Windows systems. The root problem stemmed from drive letter paths being treated as file URL paths, leading to internal errors that couldn't be fixed by external logic.

In addition, we refactored the ls function to handle file URL paths more effectively. Relative file URL paths, such as 'file:./' or 'file:../', are no longer supported and have been removed to ensure compliance with the WHATWG URL Standard specification.

Overall, these adjustments aim to bolster the project's reliability and functionality on Windows systems while simultaneously improving code maintainability.

Note

These changes have been thoroughly tested on Windows operating systems using both PowerShell and MinGW shell environments to ensure compatibility and reliability.

In this change, refactored the local `fileUrlToPath` and `ls` function on extracting the string path from given URL object. Utilizing the `url.fileURLToPath` to ensure it extracted correctly in Windows instead of extract directly from `pathname` property which add trailing slash ('/') before the drive letter. For instance:

Output using `pathname` property:
  "/D:/a/b/c"

Output using `url.fileURLToPath`:
  "D:\\a\\b\\c"

  Combined with path separator replacement:
    "D:/a/b/c"
Added two constant variables containing regular expression pattern to parse file URL and Windows path, and added a new function called `isWin32Path` to detects whether the given path is a Windows path.
Added a new function to resolve file URL path to a file path. The given path should be a valid file URL following the WHATWG URL Standard.

If the provided URL is 'file://' or 'file:///', it is replaced with the root directory path (in the current drive for Windows systems). Otherwise, the URL is parsed using the `fileURLToPath` function.
Fixed and enhanced the Windows path resolution on `ls` function. Where in Windows systems the function cannot resolve the absolute path, because the drive letter path are treated as file URL path and causing an error.
Deprecated the `fileUrlToPath` function and re-sorted the import statements.
Fixed the error while attempting to stat an entry that are read-protected due to a permission error (EPERM) or access error (EACCES).

If throws an error during stating, then it attempts to open the entry using `fs.opendir` , if throw an error with code ENOTDIR we can coclude that the entry is a regular file, otherwise, it is a directory. We also not forget to close the `Dir` instance to avoid unexpected behavior. In addition, we also sort the result entries exact before it being returned.
- Added a parameter to `TestError` constructor, it accepts an object.
- In the `it` function, changed the error message and passed the cause error to the `cause` field in `opts` parameter.
- `prepublishOnly`: Builds the project with '--overwrite' argument.
- `prepack`: Tests the project before packing the project (`npm pack`).
@mitsuki31 mitsuki31 added enhancement New feature or request minor Minor changes (e.g., introduce new features and APIs) refactor Refactor changes bugfix Bug or issue fixes labels May 21, 2024
@mitsuki31 mitsuki31 self-assigned this May 21, 2024
Now it runs on Windows to check the compatibility both on Ubuntu (Linux) and Windows systems.
@mitsuki31 mitsuki31 merged commit ea65228 into master May 21, 2024
6 checks passed
@mitsuki31 mitsuki31 deleted the refactor/enhance-support-for-windows branch May 21, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Bug or issue fixes enhancement New feature or request minor Minor changes (e.g., introduce new features and APIs) refactor Refactor changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant