Update Cmake min version and Mathcad build#55
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughRaises CMake minimum to 3.20, sets project versioning and IF97_VERSION, enforces Windows-only Mathcad Prime module with auto-detection and resource embedding, removes Visual Studio project files, and reformats Mathcad build instructions. ChangesMathcad Build System Modernization
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CMakeLists.txt (1)
14-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAllow multi-digit version components in
get_version().The current regex only accepts single-digit
major.minor.patch. As soon asIF97.hmoves to something like2.10.0, configuration will fail with a FATAL_ERROR since the pattern[0-9]\.[0-9]\.[0-9]matches only2.1and leaves.0unmatched.Suggested fix
- string(REGEX MATCH "[0-9]\\.[0-9]\\.[0-9]" SECOND_MATCH "${FIRST_MATCH}") + string(REGEX MATCH "[0-9]+\\.[0-9]+\\.[0-9]+" SECOND_MATCH "${FIRST_MATCH}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CMakeLists.txt` around lines 14 - 23, The version regex in get_version() only accepts single-digit components; update both regexes to allow multi-digit parts so versions like 2.10.0 are matched. In the file(STRINGS ...) call that currently uses "(.*IF97VERSION.*\")(v[0-9](\\.[0-9])*)(\".*)" replace the version capture with something like "v[0-9]+(\\.[0-9]+){2}" (or "v[0-9]+(\\.[0-9]+)*" if you want variable length components), and in the string(REGEX MATCH "[0-9]\\.[0-9]\\.[0-9]" SECOND_MATCH ...) replace the pattern with "[0-9]+\\.[0-9]+\\.[0-9]+" so FIRST_MATCH/SECOND_MATCH correctly capture multi-digit major/minor/patch values before set(${output} ...).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CMakeLists.txt`:
- Around line 99-101: The find_program() invocation that sets MATHCAD_EXE
currently can return cached or PATH-located executables, so unset the cached
MATHCAD_EXE before each search (use unset(MATHCAD_EXE CACHE)) and add the
NO_DEFAULT_PATH option to the find_program() calls (the ones that locate
MathcadPrime.exe) so the search is constrained to the provided PATHS; update
both occurrences tied to the IF97_PRIME_ROOT logic and the other Mathcad search
to follow this pattern.
In `@wrapper/Mathcad/README.md`:
- Around line 71-77: Remove the blank line that splits the blockquote so the
entire note and preceding quoted lines form a single contiguous blockquote;
specifically, edit the README.md block that begins with the quote lines ("Insert
your version of Mathcad Prime...", the `-A x64` note, and the line starting with
"**NOTE:**") and join them without an empty line between them to satisfy
markdownlint MD028.
- Around line 57-60: Update the two plain triple-backtick fenced blocks that
contain the shell commands "mkdir buildPrime / cd buildPrime" and "mkdir build15
/ cd build15" to include a language identifier (e.g., ```bash) so the fences
become fenced bash blocks; simply change the opening backticks for each block to
```bash to satisfy markdownlint MD040 and match the existing CMake examples.
---
Outside diff comments:
In `@CMakeLists.txt`:
- Around line 14-23: The version regex in get_version() only accepts
single-digit components; update both regexes to allow multi-digit parts so
versions like 2.10.0 are matched. In the file(STRINGS ...) call that currently
uses "(.*IF97VERSION.*\")(v[0-9](\\.[0-9])*)(\".*)" replace the version capture
with something like "v[0-9]+(\\.[0-9]+){2}" (or "v[0-9]+(\\.[0-9]+)*" if you
want variable length components), and in the string(REGEX MATCH
"[0-9]\\.[0-9]\\.[0-9]" SECOND_MATCH ...) replace the pattern with
"[0-9]+\\.[0-9]+\\.[0-9]+" so FIRST_MATCH/SECOND_MATCH correctly capture
multi-digit major/minor/patch values before set(${output} ...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: baf2ecf0-1e73-4a34-9d80-a18032cbefc5
📒 Files selected for processing (5)
CMakeLists.txtwrapper/Mathcad/IF97.slnwrapper/Mathcad/IF97.vcxprojwrapper/Mathcad/README.mdwrapper/Mathcad/dev/IF97.rc.in
💤 Files with no reviewable changes (2)
- wrapper/Mathcad/IF97.sln
- wrapper/Mathcad/IF97.vcxproj
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
wrapper/Mathcad/README.md (1)
71-77:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the note as one contiguous blockquote.
There is still a blank separator in this quoted note section (MD028). Remove the empty quoted break so the whole note remains one continuous blockquote.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wrapper/Mathcad/README.md` around lines 71 - 77, The blockquote in README.md describing Mathcad Prime options contains an empty quoted line that breaks the note into multiple blockquote paragraphs (MD028); remove the blank quoted separator so the entire NOTE (the paragraph that mentions "-DIF97_PRIME_ROOT", the Mathcad Prime root directory, and CMake behavior) is one contiguous blockquote without intervening empty quote lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@wrapper/Mathcad/README.md`:
- Around line 71-77: The blockquote in README.md describing Mathcad Prime
options contains an empty quoted line that breaks the note into multiple
blockquote paragraphs (MD028); remove the blank quoted separator so the entire
NOTE (the paragraph that mentions "-DIF97_PRIME_ROOT", the Mathcad Prime root
directory, and CMake behavior) is one contiguous blockquote without intervening
empty quote lines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a44a697-a85b-49b0-8c89-44f23a4eb4c0
📒 Files selected for processing (1)
wrapper/Mathcad/README.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Updated formatting and improved clarity in README instructions for Mathcad and Visual Studio versions.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
wrapper/Mathcad/README.md (1)
64-64: 💤 Low valueConsider using a standard language identifier.
The
cmakeidentifier is non-standard for fenced code blocks. Since this is a shell invocation of the cmake command, consider usingbashorshellfor consistent syntax highlighting.📝 Suggested change
- ```cmake + ```bash cmake .. -DIF97_PRIME_MODULE=ON🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wrapper/Mathcad/README.md` at line 64, The fenced code block in README.md currently uses the non-standard language identifier "cmake"; change that fence language to a standard shell language such as "bash" or "shell" so the line starting with the triple backticks becomes ```bash (or ```shell) to ensure correct syntax highlighting for the shell invocation `cmake .. -DIF97_PRIME_MODULE=ON`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wrapper/Mathcad/README.md`:
- Around line 71-75: Fix the markdown list indentation and remove the stray
period: adjust the three list items that currently have 4-space indentation
("Insert your version of Mathcad Prime...", "Insert your version of Visual
Studio...", "Note that Mathcad Prime...") to use the normal single-space bullet
indentation consistent with the surrounding list to satisfy markdownlint MD005,
and delete the orphaned line that contains only "." so the README.md bullets are
properly formatted and no empty period remains.
- Around line 99-101: The markdown list items "Legacy Mathcad was 32-bit and
requires a 32-bit add-in DLL." and "Prior to VS2017, use something like `-G
"Visual Studio 14 2015` as 32-bit was the default." have 4-space indentation
causing MD005; change their indentation to a single space (align them with the
other list items) so each line begins with one space then the hyphen, ensuring
consistent list indentation in README.md.
---
Nitpick comments:
In `@wrapper/Mathcad/README.md`:
- Line 64: The fenced code block in README.md currently uses the non-standard
language identifier "cmake"; change that fence language to a standard shell
language such as "bash" or "shell" so the line starting with the triple
backticks becomes ```bash (or ```shell) to ensure correct syntax highlighting
for the shell invocation `cmake .. -DIF97_PRIME_MODULE=ON`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5411c00e-5294-48ba-ab8b-ed1154e177af
📒 Files selected for processing (1)
wrapper/Mathcad/README.md
Comment on lines
+99
to
+101
| - Insert your version of Visual Studio for the -G option. | ||
| - Legacy Mathcad was 32-bit and requires a 32-bit add-in DLL. | ||
| - Prior to VS2017, use something like `-G "Visual Studio 14 2015` as 32-bit was the default. |
Contributor
There was a problem hiding this comment.
Fix inconsistent list indentation.
Lines 100-101 use 4-space indentation where 1-space is expected, triggering markdownlint MD005.
📐 Proposed fix
-
- - Insert your version of Visual Studio for the -G option.
- - Legacy Mathcad was 32-bit and requires a 32-bit add-in DLL.
- - Prior to VS2017, use something like `-G "Visual Studio 14 2015` as 32-bit was the default.
+
+ - Insert your version of Visual Studio for the -G option.
+ - Legacy Mathcad was 32-bit and requires a 32-bit add-in DLL.
+ - Prior to VS2017, use something like `-G "Visual Studio 14 2015` as 32-bit was the default.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - Insert your version of Visual Studio for the -G option. | |
| - Legacy Mathcad was 32-bit and requires a 32-bit add-in DLL. | |
| - Prior to VS2017, use something like `-G "Visual Studio 14 2015` as 32-bit was the default. | |
| - Insert your version of Visual Studio for the -G option. | |
| - Legacy Mathcad was 32-bit and requires a 32-bit add-in DLL. | |
| - Prior to VS2017, use something like `-G "Visual Studio 14 2015` as 32-bit was the default. |
🧰 Tools
🪛 LanguageTool
[style] ~101-~101: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...d requires a 32-bit add-in DLL. - Prior to VS2017, use something like `-G "Visual ...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
🪛 markdownlint-cli2 (0.22.1)
[warning] 100-100: Inconsistent indentation for list items at the same level
Expected: 1; Actual: 4
(MD005, list-indent)
[warning] 101-101: Inconsistent indentation for list items at the same level
Expected: 1; Actual: 4
(MD005, list-indent)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wrapper/Mathcad/README.md` around lines 99 - 101, The markdown list items
"Legacy Mathcad was 32-bit and requires a 32-bit add-in DLL." and "Prior to
VS2017, use something like `-G "Visual Studio 14 2015` as 32-bit was the
default." have 4-space indentation causing MD005; change their indentation to a
single space (align them with the other list items) so each line begins with one
space then the hyphen, ensuring consistent list indentation in README.md.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of the Change
Several Updates to CMake
Benefits
Possible Drawbacks
None. No code changes to the IF97 library itself.
Verification Process
Applicable Issues
Same as issue CoolProp/CoolProp#2391
Summary by CodeRabbit
Chores
Improvements
Documentation