Skip to content

Add 'is_absolute_path()' function#133

Merged
dacap merged 1 commit intoaseprite:mainfrom
Gasparoken:add-is-absolute-path
Apr 1, 2025
Merged

Add 'is_absolute_path()' function#133
dacap merged 1 commit intoaseprite:mainfrom
Gasparoken:add-is-absolute-path

Conversation

@Gasparoken
Copy link
Member

@Gasparoken Gasparoken commented Mar 19, 2025

Part of the solution of issue aseprite/aseprite#5066

@dacap
Copy link
Member

dacap commented Mar 21, 2025

Please check that this PR now contains a commit with changes I've made in bd36a11, and I'm the author (?)

@dacap dacap assigned Gasparoken and unassigned dacap Mar 21, 2025
@dacap
Copy link
Member

dacap commented Mar 21, 2025

Another comment: it looks like the get_absolute_path() impl on Windows (defined in base/fs_win32.h) (and probably the one for Unix-like systems in base/fs_unix.h) could use this function to detect if the given path is absolute.

@Gasparoken Gasparoken force-pushed the add-is-absolute-path branch 4 times, most recently from 8412855 to f9ebb90 Compare March 24, 2025 20:41
@Gasparoken Gasparoken assigned dacap and unassigned Gasparoken Mar 24, 2025
base/fs.cpp Outdated

while (itFrom != baseDirs.end() && itTo != toParts.end() && *itFrom == *itTo) {
while (itFrom != baseDirs.end() && itTo != toParts.end() &&
base::string_to_lower(*itFrom) == base::string_to_lower((*itTo))) {
Copy link
Member

Choose a reason for hiding this comment

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

We cannot do this on Linux (and sometimes depending on the configuration of the macOS file system), but I'd be worried only for Linux which the file system is case sensitive.

@dacap dacap assigned Gasparoken and unassigned dacap Mar 26, 2025
@Gasparoken Gasparoken force-pushed the add-is-absolute-path branch 4 times, most recently from 3651931 to eedf10c Compare March 27, 2025 14:05
base/fs.cpp Outdated
#if LAF_MACOS
static bool* g_isfilesystemCaseSensitive = nullptr;

bool isFilesystemCaseSensitive() {
Copy link
Member

Choose a reason for hiding this comment

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

Please use snake_case for global/non-member functions.

base/fs.cpp Outdated
std::string path1 = get_current_path() + "/filesystemTest.txt";
std::string path2 = get_current_path() + "/FilesystemTest.TXT";

std::ofstream file1(path1);
Copy link
Member

Choose a reason for hiding this comment

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

This can fail in a lot of different ways (e.g. we don't have access to write in the current path). I'd try to avoid checking this on macOS, or investigate the correct way to check if the file system is case sensitive (it might vary from file system/mount point, so I'm not sure if I'd worry about this).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. In conclusion, should we keep the macOS case-sensitive format as it was originally? Or adopt the new case-insensitive file name comparison, as described in the first pull request?

Copy link
Member

Choose a reason for hiding this comment

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

I think something like 3651931 is enough, Linux being the only platform where case-sensitive is allowed, and Windows/macOS case-insensitive.

@Gasparoken Gasparoken force-pushed the add-is-absolute-path branch 4 times, most recently from 6adc5d8 to 3432af9 Compare March 27, 2025 18:28
@Gasparoken Gasparoken force-pushed the add-is-absolute-path branch from 3432af9 to bc163a0 Compare March 27, 2025 19:01
Also, the 'get_relative_path(pathA, pathB)' function has been made
case-insensitive. This is done to correctly calculate common folders
in case one of the entered paths has a case difference.
@Gasparoken Gasparoken force-pushed the add-is-absolute-path branch from bc163a0 to b311782 Compare March 27, 2025 19:04
@Gasparoken Gasparoken assigned dacap and unassigned Gasparoken Mar 27, 2025
@dacap dacap merged commit 12df891 into aseprite:main Apr 1, 2025
9 checks passed
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.

2 participants

Comments