这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@scribblemaniac
Copy link
Member

This PR creates a new function closestCanonicalPath that behaves like QDir::canonicalPath, except that it will try to resolve symlinks for the nearest existing directory in the path if the path passed as an argument does not exist. For example, if /symlink points to the directory /target, then QDir("/symlink/example.txt").canonicalPath() and closestCanonicalPath("/symlink/example.txt") will both return /target/example.txt if example.txt exists. If it does not exist, then QDir::canonicalPath will output an empty string while this new function will still output /target/example.txt.

The motivation for this new function is twofold. It can be used to greatly simplify the implementation of validateDataPath, which was much more complex that it would seem on first glance and I suspect did not handle certain edge cases correctly. Secondly, this new function can be used in some unit tests where comparing file path strings was failing because one or more of the directories were symlinks. Existing canonical path functions could not be used because the actual files were not created for the tests.

This supersedes the changes made in #1928 to try to fix the unit tests. In my opinion, validateDataDir is not the correct function to use in that situation. The intent of validateDataDir is merely to make sure that project file paths do not refer to locations outside of the data directory. But for the failing asserts, what we want to test is that the file path stored in the keyframes points to where the actual files would be located if it was a real project. Equivalent filesystem paths should always resolve to the same string with closestCanonicalPath and thus should be safe to use string comparison with.

Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

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

LGTM

I like the simplification of validateDataPath as it indeed was quite complex.

Will these changes be cherry picked for the release branch or will you simply swap to 0.7.1?

Tested on macOS 15.6.1 (24G90)

@chchwy chchwy merged commit cd98e1b into pencil2d:master Sep 22, 2025
7 of 8 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Pull Request Priority Sep 22, 2025
chchwy pushed a commit to chchwy/pencil2d that referenced this pull request Sep 22, 2025
* Refactor validateDataPath
* Fix tests when the data dir path has symlinks
@chchwy chchwy added this to the 0.7.1 milestone Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

3 participants