Skip to content

Conversation

mmahrouss
Copy link
Collaborator

No description provided.

@mmahrouss mmahrouss added this to the 0.10.0 milestone Jul 25, 2025
@mmahrouss mmahrouss requested a review from fda-odoo July 25, 2025 13:19
@mmahrouss mmahrouss self-assigned this Jul 25, 2025
@mmahrouss mmahrouss added bug Something isn't working enhancement New feature or request labels Jul 25, 2025
@@ -634,7 +634,8 @@ F: Fn(&String) -> bool,
let config_dir = config_path.parent().map(PathBuf::from).unwrap_or_else(|| PathBuf::from("."));
if has_template(&sourced_path.value) {
return fill_validate_path(ws_folders, workspace_name, &sourced_path.value, predicate, var_map, &config_dir)
.map(|p| PathBuf::from(p).sanitize())
.and_then(|p| std::fs::canonicalize(PathBuf::from(p)).ok())
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the goal of the canonicalize?
I tried using it earlier in the project, but it was too inconsistent, or can completely change your path to get you a valid one. Moreover it has a high performance cost

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am doing to get an absolute path. to not have /../ in the path like here
image

it has a high cost, but it is run once just when initing. Do you think that would be considerable?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, here performance cost is not so important.
However I'm more afrait about the exactness of the result. With symlink for example. But I guess that would not be an issue for now as we don't support symlink at all

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@fda-odoo cannonicalize is also supposed to resolve symbolic links

@fda-odoo fda-odoo merged commit 1689870 into master Aug 6, 2025
2 checks passed
@mmahrouss mmahrouss deleted the canonicalize-pattern-filled-paths branch August 14, 2025 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants