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

Cleanup: move pl.path, pl.file and pl.dir into a single module #416

Open
Tieske opened this issue Feb 13, 2022 · 5 comments
Open

Cleanup: move pl.path, pl.file and pl.dir into a single module #416

Tieske opened this issue Feb 13, 2022 · 5 comments

Comments

@Tieske
Copy link
Member

Tieske commented Feb 13, 2022

All of these modules have overlapping functionalities wrt to the filesystem

@tst2005
Copy link
Contributor

tst2005 commented Feb 15, 2022

hello @Tieske
3 years ago I tried to refactor Penlight, probably what you are doing now.
At first, thanks for the work done!

I'm agree all these 3 modules are related to file system features.

I check the dependency graph (made 3 years ago) for pl.dir and pl.file require pl.path.

pl.path was used by :

  • pl.app
  • pl.dir
  • pl.file
  • pl.test

pl.dir was used by :

  • pl.file

pl.file was not used by other pl modules

I don't know if these informations are still up-to-date. maybe it should help you ... or not.

Regards,

@RiskoZoSlovenska
Copy link
Contributor

Hello,
I'd be willing to spend some time on this. I'd like to propose the following changes:

Click to expand
  • fs -> New module
    • remove <- os.remove
    • link <- lfs.link
    • touch <- lfs.touch
    • path <- a submodule/table holding path-related methods
  • file -> Remove
  • utils
    • readfile -> fs.readfile
    • readlines -> fs.readlines
    • writefile -> fs.writefile
  • dir
    • fnmatch -> fs.path.match
    • filter -> fs.path.filter
    • getfiles -> fs.getfiles
    • getdirectories -> fs.getdirectories
    • copyfile -> fs.copyfile
    • movefile -> fs.movefile
    • walk -> fs.walk
    • rmtree -> fs.rmtree
    • makepath -> fs.mkdirp
    • clonetree -> fs.clonetree
    • dirtree -> fs.dirtree
    • getallfiles -> fs.getallfiles
  • path
    • dir -> Remove (use fs.walk or fs.dirtree)
    • rmdir -> Remove (use fs.rmtree)
    • mkdir -> fs.mkdir
    • attrib -> fs.attrib
    • link_attrib -> fs.linkattrib
    • currentdir -> fs.currentdir
    • chdir -> fs.chdir
    • isdir -> Remove (use fs.attrib) (same goes for the following methods)
    • isfile -> Remove
    • getsize -> Remove
    • getatime -> Remove
    • getmtime -> Remove
    • getctime -> Remove
    • exists -> fs.exists
    • splitpath -> fs.path.split
    • abspath -> fs.path.abs
    • splitext -> fs.path.splitext
    • dirname -> fs.path.dirname
    • basename -> fs.path.basename
    • extension -> fs.path.extname
    • isabs -> fs.path.isabs
    • join -> fs.path.join
    • normcase -> fs.path.normcase
    • normpath -> fs.path.norm
    • relpath -> fs.path.rel
    • expanduser -> fs.expanduser
    • tmpname -> fs.tmpname
    • common_prefix -> fs.path.commonprefix
    • package_path -> fs.path.packagepath

The idea is that fs.path (a part of fs) will contain methods which simply operate on Lua strings and don't make any system calls, while the rest of the fs module will hold the main filesystem functions.

@Tieske
Copy link
Member Author

Tieske commented Mar 7, 2024

A side discussion that we had was whether we should split the modules for Unix-like, MacOS and Windows, since the path handling is different.

  • Windows has drive letters as well as network paths, uses \ instead of / as separator
  • MacOS and Windows are case insensitive for finding files (but retain casing when creating)
  • Unix is case sensitive

@alerque probably also has some opinions on this.

@RiskoZoSlovenska
Copy link
Contributor

I'm taking a lot of inspiration from https://github.com/truemedian/luvit-api-design, and I think the way it handles paths is pretty good: the path table/module is itself split up into path.posix and path.windows, and then function aliases for the current platform are inserted into the base path table when the library is loaded. For example, path.posix.abs and path.windows.abs are separate implementations, and path.abs is then set dynamically based on the current platform.

@Tieske
Copy link
Member Author

Tieske commented Mar 8, 2024

That's a similar approach to LuaRocks, with the overriding modules: https://github.com/luarocks/luarocks/tree/master/src/luarocks/fs

Though posix and windows alone is not enough. Mac has different behaviour because of its case insensitive treatment of files. So listing file matching README.* should return readme.md on windows and mac, but not on posix.

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

No branches or pull requests

3 participants