Closes Bug #1273 mac os dcm2nii error#1966
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes macOS dcm2nii error handling by standardizing path quoting across Unix-based systems and removing macOS-specific command line handling that was causing issues.
- Standardized path quoting in system commands across Unix/Linux/macOS platforms
- Removed macOS-specific command line formatting for dcm2nii operations
- Added validation warnings for empty JSON fields and improved testing infrastructure
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Testing/xASL_test_Flavors.m | Fixed typo, added screen clearing, and updated delete function call |
| Functions/InputOutput/xASL_io_dcm2nii.m | Removed macOS-specific command line handling to use standardized quoting |
| Functions/BIDS/xASL_bids_BIDSifyASLJSON.m | Added validation for empty fields in JSON and studyPar structures |
| External/SPMmodified/xASL/xASL_SysMove.m | Standardized path quoting for Unix systems and commented out path conversion |
| External/SPMmodified/xASL/xASL_SysCopy.m | Standardized path quoting for Unix systems and commented out path conversion |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| system(['cp -r -f ' quote SrcPath quote '/* ' quote DstPath quote]); % -n is short for --noclober | ||
| else | ||
| system(['cp -r ' SrcPath '/* ' DstPath]); | ||
| system(['cp -r ' quote SrcPath '/*' quote ' ' quote DstPath quote]); |
There was a problem hiding this comment.
The quoting for the wildcard pattern is incorrect. The '/*' should be outside the quotes to be interpreted by the shell, not inside. This should be: system(['cp -r ' quote SrcPath quote '/* ' quote DstPath quote]);
| system(['cp -r ' quote SrcPath '/*' quote ' ' quote DstPath quote]); | |
| system(['cp -r ' quote SrcPath quote '/* ' quote DstPath quote]); |
jan-petr
left a comment
jan-petr
left a comment
There was a problem hiding this comment.
I reviewed what's the until now and fine with all. I will run flavor tests once the issue is finalized.
Linked issue
Closes #1273
How to test
Required: if not defined in the linked issue, add a simple test description here
Comments
Optional: add helpful comments for the reviewers here