Recursive is off by default for symbolic folders#206
Conversation
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[test] | ||
| fn setting_recurse_true() { | ||
| let global: GlobalConfig = toml::from_str( | ||
| r#" | ||
| [settings] | ||
| recurse = true | ||
|
|
||
| [cat] | ||
| depends = [] | ||
|
|
||
| [cat.files] | ||
| cat = '~/.QuarticCat' | ||
| "#, | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| let local: LocalConfig = toml::from_str( | ||
| r#" | ||
| packages = ['cat'] | ||
| "#, | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| let merged_config = merge_configuration_files(global, local, None).unwrap(); | ||
|
|
||
| assert!(merged_config.recurse); | ||
| } |
There was a problem hiding this comment.
Consider adding a test case for explicitly setting recurse = false to ensure all three cases are covered: default (false), explicit true, and explicit false. This would make the test suite more comprehensive and verify that the setting works correctly in all scenarios.
|
I am surprised that aside from this PR I cant find any toggle that enables directory level symlinks. Without this feature I'm stuck shell scripting. |
ConcurAc
left a comment
ConcurAc
left a comment
There was a problem hiding this comment.
I would be inclined to keep the default for recursion as true.
Switching recurse to false will introduce friction for users migrating between version, which seems unnecessary to me.
I think this makes more sense and I believe it is the expected behavior. I also added an option in the settings to turn it on/off.