search

bcpeinhardt / simplifile

Simple file operations for Gleam that work on all targets (Erlang/Node/Deno)
71 stars 10 forks source link

is_file and is_direcory silently discard errors and return incorrect information #20

Closed lpil closed 7 months ago

lpil commented 7 months ago

Hello!

They return Bool rather than Result(Bool, Error), so any errors get silently discarded and an incorrect result is returned.

https://hexdocs.pm/simplifile/simplifile.html#is_file

https://hexdocs.pm/simplifile/simplifile.html#is_directory

Thanks, Louis

bcpeinhardt commented 7 months ago

Yeah, it looks like they're inconsistently implemented as well 😂. Think I should deprecate them and add new ones or just go ahead to v2?

bcpeinhardt commented 7 months ago

Ugh. Erlang is perfectly happy to give me information about "/etc" even when I shouldn't have permission, node not so much. But I guess that's okay as long as there's no false positives? Frustrating though.

bcpeinhardt commented 7 months ago

AHA never mind all that it wasn't permissions it was me using lstatSync instead of statSync haha.

lpil commented 7 months ago

Deprecating and giving folks time to upgrade could be good, but what would they be called?

bcpeinhardt commented 7 months ago

I've got everything implemented and cleaned up :) Took me forever to figure out fs.existsSync also silently returns false haha. Right now I'm calling them is_valid_file and is_valid_directory. But maybe something like verify_is_file would be better? That way it's clear the error isn't with the file or dir, but with the verification (because the common error here would be permissions)

bcpeinhardt commented 7 months ago

Had to run but didn't want to leave you hanging without these changes so I went with verify_is_file and verify_is_directory :)