-
Notifications
You must be signed in to change notification settings - Fork 121
Homebrew formula #1037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Homebrew formula #1037
Changes from all commits
b60f27c
007e05e
95097c9
edfc8fe
c1b5453
e34711d
6ee36b7
33e9645
7d21ed3
35b4469
bd9f51e
dd0b8c4
07ee05e
29a928d
d1d1c67
b2e034e
86a4c89
79924a4
b7c580c
919a800
9fd8391
d176487
5f5ced5
d372e63
3c5d327
161d41d
52de223
1d3bc21
3b8c950
46e325a
a38350f
272340a
9b8db76
5a48e36
5d7ef61
b8a2940
9a45f65
4e5cc79
1cd2cc3
5d2eb66
e2c249c
4edc60f
55ca2b0
f9d3409
24452d4
26df743
dd0ba7c
3a4ce68
301ea0b
f3643b9
ef4aba5
7dc3c9a
09fe44e
19fe2d9
b1c75bb
1612552
3973747
d4dcaaa
23949ef
0385aa8
00f8295
7f34c7e
abe825a
4bffab5
bd376b4
f4fa72e
b8c2463
06edb9b
9e21f25
5ee24d4
62393d1
69427d9
b9d4148
96fe6d4
5c9bbb7
9163b77
ed9c328
0d472e7
5a7a31a
8de2268
bc7175f
c56cd92
cae22fd
09c8bf2
385061a
692519f
1408aec
5b85475
ef66c9a
d3b1a93
e75b7ff
415ad05
8a75271
d1381c8
3efb9d6
845c68b
a7572e2
2308b9a
3be3e15
1a32884
6b5bd9f
5da2c3a
8215624
3ff41d1
72304d5
43bfb7b
1bb2226
315c4fd
f7e6aea
81e30ff
bb096d7
efb6c53
93f8c3b
2db4c31
59f196b
0aab6ca
66e2093
323e3bf
11a4e98
c3b0c1d
95f2109
ede409c
dcc6031
addcd87
25a39df
9d4e299
e3a7f3c
2b2b749
cf453b9
f5f9abb
b1e3ac5
c73b091
9ade107
6970bce
61adc7e
1e78657
cb29fc5
7c7543f
4041006
346c413
ec24979
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -102,10 +102,12 @@ def install | |||||||||||||||||||||||||||||||||
| MFC (Homebrew) #{version} | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Usage: | ||||||||||||||||||||||||||||||||||
| mfc <case.py> [options] | ||||||||||||||||||||||||||||||||||
| mfc run <case.py> [options] | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Examples: | ||||||||||||||||||||||||||||||||||
| mfc run case.py -j 1 | ||||||||||||||||||||||||||||||||||
| mfc case.py -n 2 | ||||||||||||||||||||||||||||||||||
| mfc run case.py -n 2 -t pre_process simulation | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Notes: | ||||||||||||||||||||||||||||||||||
| - This Homebrew wrapper uses prebuilt binaries and a preinstalled venv. | ||||||||||||||||||||||||||||||||||
|
|
@@ -115,9 +117,15 @@ def install | |||||||||||||||||||||||||||||||||
| exit 0 | ||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Smart detection: if first arg looks like a case file, auto-prepend "run" | ||||||||||||||||||||||||||||||||||
| if [[ "${SUBCMD}" =~ .py$ ]] || [[ -f "${SUBCMD}" ]]; then | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| if [[ "${SUBCMD}" =~ .py$ ]] || [[ -f "${SUBCMD}" ]]; then | |
| if [[ "${SUBCMD}" =~ \.py$ ]] || [[ -f "${SUBCMD}" ]]; then |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file existence check [[ -f "${SUBCMD}" ]] is too broad and will trigger auto-detection for any file, not just Python case files. This means commands like mfc myfile.txt would incorrectly be transformed to mfc run myfile.txt.
Consider making the check more specific, e.g., [[ "${SUBCMD}" =~ \.py$ ]] || [[ "${SUBCMD}" =~ \.py$ && -f "${SUBCMD}" ]], or simply rely only on the file extension check since Python case files should always end in .py.
| if [[ "${SUBCMD}" =~ .py$ ]] || [[ -f "${SUBCMD}" ]]; then | |
| if [[ "${SUBCMD}" =~ \.py$ ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Modify the case file detection logic to handle command-line options placed before the filename. The current implementation only checks the first argument, which fails in this common scenario. [possible issue, importance: 7]
| # Smart detection: if first arg looks like a case file, auto-prepend "run" | |
| if [[ "${SUBCMD}" =~ .py$ ]] || [[ -f "${SUBCMD}" ]]; then | |
| ARGS=("run" "${ARGS[@]}") | |
| SUBCMD="run" | |
| fi | |
| # Smart detection: if 'run' is not specified, find a case file and prepend 'run' | |
| if [[ "${SUBCMD}" != "run" ]]; then | |
| for arg in "${ARGS[@]}"; do | |
| # If we find an argument that is a file and not an option, prepend "run" | |
| if [[ ! "${arg}" =~ ^- ]] && ([[ "${arg}" =~ \.py$ ]] || [[ -f "${arg}" ]]); then | |
| ARGS=("run" "${ARGS[@]}") | |
| SUBCMD="run" | |
| break | |
| fi | |
| done | |
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High-level Suggestion
The PR adds a shorthand for the
runsubcommand, which is inconsistent with the main developer tool's CLI. To ensure a consistent user experience, this implicit behavior should be removed, requiring the explicitmfc run <case.py>command. [High-level, importance: 7]Solution Walkthrough:
Before:
After: