Upstream sync - #23
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe startup script now accepts configurable MT5 command-line options. A Windows Docker Compose configuration adds persistent storage and exposed ports. The README documents Windows setup and MT5 option usage. ChangesMT5 configuration and Windows deployment
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoAdd Windows docker-compose and MT5 command-line option passthrough
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
There was a problem hiding this comment.
Pull request overview
This PR syncs upstream changes by improving end-user documentation, adding a Windows-focused compose example, and introducing support for passing MetaTrader 5 command-line options into the container startup.
Changes:
- Document MT5 usage more clearly (formatting, Windows-specific guidance) and add a new section describing
MT5_CMD_OPTIONS. - Add
docker-compose-windows.yamlas a Windows-friendly compose file using a Docker-managed volume and.env. - Update
Metatrader/start.shto passMT5_CMD_OPTIONSto the MT5 launch command.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| README.md | Refreshes and expands usage/configuration docs; adds Windows and MT5 command-line options guidance. |
| Metatrader/start.sh | Adds MT5_CMD_OPTIONS support when launching MT5 under Wine. |
| docker-compose-windows.yaml | Introduces a Windows-oriented compose configuration using a managed volume and .env. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if [ -e "$mt5file" ]; then | ||
| show_message "[4/7] File $mt5file is installed. Running MT5..." | ||
| $wine_executable "$mt5file" & | ||
| $wine_executable "$mt5file" $MT5_CMD_OPTIONS & | ||
| else |
| 3. Start the container | ||
| ---------- | ||
|
|
||
| **Notice**: Due to Windows permission management, if you are using windows use Docker managed volume instead of bind mount. Use this compose file instead: |
| 1. Start the container | ||
|
|
||
| ```bash | ||
| docker compose up -d | ||
| ``` | ||
|
|
||
| In some systems `docker compose` command does not exists. Try to use `docker-compose up -d` instead. | ||
|
|
||
| 4. Connect to web interface | ||
| Start your browser pointing http://<your ip address>:3000 | ||
|
|
||
| Start your browser pointing `http://<your ip address>:3000` |
| ```yaml | ||
| version: '3' | ||
| services: | ||
| mt5: | ||
| image: gmag11/metatrader5_vnc | ||
| container_name: mt5 | ||
| volumes: | ||
| - mt5_config:/config | ||
| ports: | ||
| - 3000:3000 | ||
| - 8001:8001 | ||
| environment: | ||
| - CUSTOM_USER=<Choose a user> | ||
| - PASSWORD=<Choose a secure password> | ||
|
|
||
| volumes: | ||
| mt5_config: | ||
| ``` |
Code Review by Qodo
1. MT5 options word-split
|
| if [ -e "$mt5file" ]; then | ||
| show_message "[4/7] File $mt5file is installed. Running MT5..." | ||
| $wine_executable "$mt5file" & | ||
| $wine_executable "$mt5file" $MT5_CMD_OPTIONS & |
There was a problem hiding this comment.
1. Mt5 options word-split 🐞 Bug ≡ Correctness
Metatrader/start.sh appends MT5_CMD_OPTIONS unquoted when launching MT5, so bash performs word-splitting and pathname expansion which can corrupt the intended argument list (especially for paths containing spaces or wildcards). This can cause MT5 to start without the intended /config or other parameters, breaking the new feature.
Agent Prompt
## Issue description
`MT5_CMD_OPTIONS` is expanded unquoted when invoking `wine terminal64.exe`, which triggers bash word-splitting and glob expansion.
## Issue Context
`MT5_CMD_OPTIONS` is meant to carry MT5 CLI parameters (README documents `/config:<path>`), and these paths/options can legitimately contain spaces or glob characters. Unquoted expansion can change the arguments passed to MT5.
## Fix Focus Areas
- Metatrader/start.sh[10-10]
- Metatrader/start.sh[74-74]
## Suggested fix
Use an array and a delimiter that preserves spaces inside an option (e.g., newline-separated options), then pass via `"${array[@]}"`.
Example pattern:
```bash
# Accept newline-separated options (spaces preserved)
mt5_opts=()
if [ -n "${MT5_CMD_OPTIONS:-}" ]; then
IFS=$'\n' read -r -d '' -a mt5_opts < <(printf '%s\0' "$MT5_CMD_OPTIONS")
fi
# Optionally disable globbing around this call
set -f
$wine_executable "$mt5file" "${mt5_opts[@]}" &
set +f
```
Update README to mention newline-separated options if multiple are needed.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| env_file: | ||
| - .env | ||
| volumes: | ||
| mt5-config: |
There was a problem hiding this comment.
2. Windows compose diverges docs 🐞 Bug ⚙ Maintainability
The newly added docker-compose-windows.yaml is not referenced from README and differs from the README Windows compose snippet (volume name and env configuration style), which can lead users to inadvertently create a different persistent volume or fail to provide required env vars. This increases configuration drift and setup failures for Windows users.
Agent Prompt
## Issue description
`docker-compose-windows.yaml` introduces a Windows-specific compose configuration, but README does not point to it and its defaults differ from the README Windows snippet.
## Issue Context
- README provides a Windows compose example using a named volume `mt5_config` and inline `environment:`.
- The new file uses a different volume name (`mt5-config`) and requires a `.env` via `env_file`, which is not mentioned near the Windows instructions.
## Fix Focus Areas
- docker-compose-windows.yaml[1-14]
- README.md[99-120]
## Suggested fix
Pick a single source of truth and align:
- Option A: Update README Windows section to explicitly reference `docker-compose-windows.yaml` and document creating `.env` (e.g., copy `.env.example` -> `.env`).
- Option B: Modify `docker-compose-windows.yaml` to match the README snippet (use `mt5_config` naming and inline `environment:`), or update the README snippet to match the file.
- Ensure volume naming matches to avoid users switching configs and "losing" persisted state.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docker-compose-windows.yaml`:
- Around line 6-7: Update the volume reference under the Compose service’s
volumes section from mt5-config to mt5_config, matching the Windows
documentation and preserving the existing /config mount.
In `@Metatrader/start.sh`:
- Line 74: Update the command launch around wine_executable and MT5_CMD_OPTIONS
to parse the documented separator-delimited option string into an array,
preserving each argument’s boundaries without wildcard expansion. Invoke the
executable using the resulting quoted array expansion, such as the
mt5_cmd_options symbol, rather than unquoted $MT5_CMD_OPTIONS.
In `@README.md`:
- Line 130: Correct the grammar in the Docker Compose fallback sentence by
changing “does not exists” to “does not exist,” while preserving the rest of the
instruction unchanged.
- Around line 236-238: Update the KasmVNC license references in the README to
use descriptive link text such as the license name instead of generic “here”
labels, and correct “unther” to “under” in the LinuxServer KasmVNC Base Image
paragraph.
- Line 37: Update the ordered-list entries at the referenced README locations to
use the Markdown prefix “1.” consistently, including the entry at line 132 as
the second item in the separate list after the thematic break.
- Line 127: Update the Windows setup instructions in README.md to use
docker-compose-windows.yaml explicitly when starting Compose, and document
copying .env.example to .env before launching the services.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 5c231ae0-d8a0-494a-8e48-9d186f8385ac
📒 Files selected for processing (3)
Metatrader/start.shREADME.mddocker-compose-windows.yaml
📜 Review details
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~136-~136: Ensure spelling is correct
Context: ...is automatic and you should end up with MetaTrader5 running in your web session. ## Where ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~229-~229: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...art_advanced/start). ## Contributions Feel free to contribute to this project. All contrib...
(FEEL_FREE_TO_STYLE_ME)
[grammar] ~238-~238: Ensure spelling is correct
Context: ...r/docker-baseimage-kasmvnc) is licensed unther the GNU General Public License v3.0 (GP...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.23.2)
README.md
[warning] 37-37: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 43-43: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1
(MD029, ol-prefix)
[warning] 62-62: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 132-132: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1
(MD029, ol-prefix)
[warning] 236-236: Link text should be descriptive
(MD059, descriptive-link-text)
[warning] 238-238: Link text should be descriptive
(MD059, descriptive-link-text)
🪛 Shellcheck (0.11.0)
Metatrader/start.sh
[info] 74-74: Double quote to prevent globbing and word splitting.
(SC2086)
🔇 Additional comments (3)
Metatrader/start.sh (1)
10-10: LGTM!README.md (2)
10-10: LGTM!Also applies to: 21-21, 31-35, 39-41, 56-60, 99-99, 169-169
104-104: 📐 Maintainability & Code QualityKeep
version: '3'in the README examples.The README supports
docker-composeas a fallback. Legacy Compose v1 interprets an omittedversionas format 1, which does not support theservices:structure used here. Remove these fields only if legacy Compose support is dropped.> Likely an incorrect or invalid review comment.
| volumes: | ||
| - mt5-config:/config |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'mt5-config|mt5_config' docker-compose-windows.yaml README.mdRepository: marlonsc/mt5-docker
Length of output: 437
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- docker-compose-windows.yaml ---'
cat -n docker-compose-windows.yaml
printf '%s\n' '--- README.md: Windows and volume examples ---'
sed -n '90,130p;185,225p' README.md
printf '%s\n' '--- volume identifiers and references ---'
rg -n -C 2 'mt5-config|mt5_config|docker-compose-windows|docker compose' --glob '!*.lock' .Repository: marlonsc/mt5-docker
Length of output: 4367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
compose = Path("docker-compose-windows.yaml").read_text()
readme = Path("README.md").read_text()
compose_refs = sorted(set(re.findall(r'\bmt5[-_]config\b', compose)))
readme_refs = sorted(set(re.findall(r'\bmt5[-_]config\b', readme)))
print("docker-compose-windows.yaml identifiers:", compose_refs)
print("README.md identifiers:", readme_refs)
print("Windows compose and README identifiers match:", compose_refs == readme_refs)
PY
sed -n '88,123p' README.mdRepository: marlonsc/mt5-docker
Length of output: 898
Use mt5_config for both volume references.
Rename mt5-config in docker-compose-windows.yaml to match the Windows documentation. Different identifiers select different named volumes and can hide existing MT5 state.
🤖 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 `@docker-compose-windows.yaml` around lines 6 - 7, Update the volume reference
under the Compose service’s volumes section from mt5-config to mt5_config,
matching the Windows documentation and preserving the existing /config mount.
| if [ -e "$mt5file" ]; then | ||
| show_message "[4/7] File $mt5file is installed. Running MT5..." | ||
| $wine_executable "$mt5file" & | ||
| $wine_executable "$mt5file" $MT5_CMD_OPTIONS & |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'MT5_CMD_OPTIONS|wine_executable.*mt5file' Metatrader/start.sh README.md
MT5_CMD_OPTIONS='/config:C:\Program Files\tester.ini /login:123'
printf 'Current expansion:\n'
printf '<%s>\n' $MT5_CMD_OPTIONSRepository: marlonsc/mt5-docker
Length of output: 741
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Metatrader/start.sh ---'
sed -n '1,95p' Metatrader/start.sh
printf '%s\n' '--- README.md option documentation ---'
sed -n '175,225p' README.md
printf '%s\n' '--- repository references ---'
rg -n --glob '!**/.git/**' 'MT5_CMD_OPTIONS|wine_executable|mt5file' .Repository: marlonsc/mt5-docker
Length of output: 7236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
bash <<'BASH'
set -u
show_current() {
local MT5_CMD_OPTIONS=$1
printf 'current input=%q\n' "$MT5_CMD_OPTIONS"
printf 'current argv:'
printf '<%s>' $MT5_CMD_OPTIONS
printf '\n'
}
show_array() {
local MT5_CMD_OPTIONS=$1
local -a mt5_cmd_options=()
if [[ -n "$MT5_CMD_OPTIONS" ]]; then
IFS='|' read -r -a mt5_cmd_options <<< "$MT5_CMD_OPTIONS"
fi
printf 'array input=%q\n' "$MT5_CMD_OPTIONS"
printf 'array argc=%d argv:' "${`#mt5_cmd_options`[@]}"
printf '<%s>' "${mt5_cmd_options[@]}"
printf '\n'
}
show_current '/config:C:\Program Files\tester.ini /login:123'
show_array '/config:C:\Program Files\tester.ini|/login:123'
show_current '*.md'
show_array '*.md|/login:123'
show_current ''
show_array ''
BASHRepository: marlonsc/mt5-docker
Length of output: 610
Preserve MT5_CMD_OPTIONS argument boundaries.
The unquoted expansion splits options that contain spaces and expands wildcards to filenames. Parse a documented separator-delimited format into an array, then use "${mt5_cmd_options[@]}".
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 74-74: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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 `@Metatrader/start.sh` at line 74, Update the command launch around
wine_executable and MT5_CMD_OPTIONS to parse the documented separator-delimited
option string into an array, preserving each argument’s boundaries without
wildcard expansion. Invoke the executable using the resulting quoted array
expansion, such as the mt5_cmd_options symbol, rather than unquoted
$MT5_CMD_OPTIONS.
Source: Linters/SAST tools
| cd MetaTrader5-Docker-Image | ||
| ``` | ||
|
|
||
| 2. Build the Docker image: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '^[[:space:]]*[0-9]+\.' README.md
if command -v markdownlint-cli2 >/dev/null 2>&1; then
markdownlint-cli2 README.md
fiRepository: marlonsc/mt5-docker
Length of output: 2926
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README list context ---'
nl -ba README.md | sed -n '24,70p;112,140p'
printf '%s\n' '--- markdownlint configuration ---'
find . -maxdepth 2 -type f \( -iname '*markdownlint*' -o -name '.markdownlint*' \) -print
for f in $(find . -maxdepth 2 -type f \( -iname '*markdownlint*' -o -name '.markdownlint*' \)); do
printf '\n--- %s ---\n' "$f"
cat "$f"
doneRepository: marlonsc/mt5-docker
Length of output: 225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README list context ---'
awk 'NR >= 24 && NR <= 70 || NR >= 112 && NR <= 140 { printf "%d:%s\n", NR, $0 }' README.md
printf '%s\n' '--- markdownlint configuration ---'
find . -maxdepth 2 -type f \( -iname '*markdownlint*' -o -name '.markdownlint*' \) -print
while IFS= read -r f; do
printf '\n--- %s ---\n' "$f"
cat "$f"
done < <(find . -maxdepth 2 -type f \( -iname '*markdownlint*' -o -name '.markdownlint*' \))Repository: marlonsc/mt5-docker
Length of output: 2420
Use 1. for each ordered-list item.
Change Lines 37, 43, 62, and 132 to use the configured 1. prefix. Line 124 starts a separate list after the thematic break, so Line 132 must also use 1..
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 37-37: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
🤖 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 `@README.md` at line 37, Update the ordered-list entries at the referenced
README locations to use the Markdown prefix “1.” consistently, including the
entry at line 132 as the second item in the separate list after the thematic
break.
Source: Linters/SAST tools
| 1. Start the container | ||
|
|
||
| ```bash | ||
| docker compose up -d |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'docker-compose-windows\.yaml|docker compose up -d|env_file|mt5_config|mt5-config' \
README.md docker-compose-windows.yaml docker-compose.yamlRepository: marlonsc/mt5-docker
Length of output: 554
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README.md lines 90-135 ---'
sed -n '90,135p' README.md
printf '%s\n' '--- README.md lines 175-225 ---'
sed -n '175,225p' README.md
printf '%s\n' '--- docker-compose.yaml ---'
cat -n docker-compose.yaml
printf '%s\n' '--- docker-compose-windows.yaml ---'
cat -n docker-compose-windows.yaml
printf '%s\n' '--- environment-file references ---'
rg -n --hidden --glob '!*.lock' '(^|/)\.env($|[^A-Za-z])|env_file|MT5_|mt5-config|mt5_config' .Repository: marlonsc/mt5-docker
Length of output: 3953
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README.md setup instructions ---'
sed -n '1,90p' README.md
printf '%s\n' '--- .env.example ---'
if [ -f .env.example ]; then
cat -n .env.example
else
printf '%s\n' '.env.example is absent'
fi
printf '%s\n' '--- Compose-related documentation references ---'
rg -n -i 'compose|windows|\.env|environment|CUSTOM_USER|PASSWORD' README.mdRepository: marlonsc/mt5-docker
Length of output: 4726
Select the Windows Compose file explicitly.
The Windows instructions require the named-volume configuration, but docker compose up -d loads the default bind-mounted configuration. Use docker compose -f docker-compose-windows.yaml up -d and document copying .env.example to .env.
🤖 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 `@README.md` at line 127, Update the Windows setup instructions in README.md to
use docker-compose-windows.yaml explicitly when starting Compose, and document
copying .env.example to .env before launching the services.
| docker compose up -d | ||
| ``` | ||
|
|
||
| In some systems `docker compose` command does not exists. Try to use `docker-compose up -d` instead. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the Compose fallback sentence.
Change does not exists to does not exist. (raw.githubusercontent.com)
Proposed fix
-In some systems `docker compose` command does not exists.
+In some systems the `docker compose` command does not exist.🤖 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 `@README.md` at line 130, Correct the grammar in the Docker Compose fallback
sentence by changing “does not exists” to “does not exist,” while preserving the
rest of the instruction unchanged.
| The [**KasmVNC**](https://github.com/kasmtech/KasmVNC) project is licensed under the [GNU General Public License v2.0 (GPLv2)](https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html). You can check the license details of KasmVNC [here](https://github.com/kasmtech/KasmVNC/blob/master/LICENSE.TXT). | ||
|
|
||
| [**KasmVNC Base Image from LinuxServer**](https://github.com/linuxserver/docker-baseimage-kasmvnc) is licensed unther the GNU General Public License v3.0 (GPLv3). License is available [here](https://github.com/linuxserver/docker-baseimage-kasmvnc/blob/master/LICENSE) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use descriptive license links and correct the spelling.
Replace the generic here labels with the license names. Change unther to under. These changes improve link accessibility and remove a user-facing typo. (raw.githubusercontent.com)
Proposed fix
-[here]
+[KasmVNC license file]
-licensed unther
+licensed under
-[here]
+[KasmVNC Base Image license file]📝 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.
| The [**KasmVNC**](https://github.com/kasmtech/KasmVNC) project is licensed under the [GNU General Public License v2.0 (GPLv2)](https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html). You can check the license details of KasmVNC [here](https://github.com/kasmtech/KasmVNC/blob/master/LICENSE.TXT). | |
| [**KasmVNC Base Image from LinuxServer**](https://github.com/linuxserver/docker-baseimage-kasmvnc) is licensed unther the GNU General Public License v3.0 (GPLv3). License is available [here](https://github.com/linuxserver/docker-baseimage-kasmvnc/blob/master/LICENSE) | |
| The [**KasmVNC**](https://github.com/kasmtech/KasmVNC) project is licensed under the [GNU General Public License v2.0 (GPLv2)](https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html). You can check the license details of KasmVNC [KasmVNC license file](https://github.com/kasmtech/KasmVNC/blob/master/LICENSE.TXT). | |
| [**KasmVNC Base Image from LinuxServer**](https://github.com/linuxserver/docker-baseimage-kasmvnc) is licensed under the GNU General Public License v3.0 (GPLv3). License is available [KasmVNC Base Image license file](https://github.com/linuxserver/docker-baseimage-kasmvnc/blob/master/LICENSE) |
🧰 Tools
🪛 LanguageTool
[grammar] ~238-~238: Ensure spelling is correct
Context: ...r/docker-baseimage-kasmvnc) is licensed unther the GNU General Public License v3.0 (GP...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.23.2)
[warning] 236-236: Link text should be descriptive
(MD059, descriptive-link-text)
[warning] 238-238: Link text should be descriptive
(MD059, descriptive-link-text)
🤖 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 `@README.md` around lines 236 - 238, Update the KasmVNC license references in
the README to use descriptive link text such as the license name instead of
generic “here” labels, and correct “unther” to “under” in the LinuxServer
KasmVNC Base Image paragraph.
Source: Linters/SAST tools
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docker-compose-windows.yaml">
<violation number="1" location="docker-compose-windows.yaml:7">
P3: The managed volume is named `mt5-config` (hyphen) here, but the README's Windows configuration specifies `mt5_config` (underscore) both as the mount and in the top-level volumes section. It works internally since the name is consistent, but it deviates from the documented convention and will confuse anyone cross-referencing the README or scripts that expect `mt5_config`. Consider renaming to `mt5_config` for consistency.</violation>
</file>
<file name="Metatrader/start.sh">
<violation number="1" location="Metatrader/start.sh:74">
P2: MT5 command-line options containing spaces or shell-special characters are not passed as a reliable argument list because `$MT5_CMD_OPTIONS` is unquoted. Quote the expansion if this variable represents one option string, or accept an array/structured argument interface if multiple options must be preserved independently.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:188">
P2: Command-line options containing spaces or shell-special characters will not reach MetaTrader 5 as documented because `MT5_CMD_OPTIONS` is expanded unquoted by the startup script. Please document that only shell-word-safe values are supported or change the launcher to parse/pass options safely (for example, an array-based interface).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if [ -e "$mt5file" ]; then | ||
| show_message "[4/7] File $mt5file is installed. Running MT5..." | ||
| $wine_executable "$mt5file" & | ||
| $wine_executable "$mt5file" $MT5_CMD_OPTIONS & |
There was a problem hiding this comment.
P2: MT5 command-line options containing spaces or shell-special characters are not passed as a reliable argument list because $MT5_CMD_OPTIONS is unquoted. Quote the expansion if this variable represents one option string, or accept an array/structured argument interface if multiple options must be preserved independently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Metatrader/start.sh, line 74:
<comment>MT5 command-line options containing spaces or shell-special characters are not passed as a reliable argument list because `$MT5_CMD_OPTIONS` is unquoted. Quote the expansion if this variable represents one option string, or accept an array/structured argument interface if multiple options must be preserved independently.</comment>
<file context>
@@ -70,7 +71,7 @@ fi
if [ -e "$mt5file" ]; then
show_message "[4/7] File $mt5file is installed. Running MT5..."
- $wine_executable "$mt5file" &
+ $wine_executable "$mt5file" $MT5_CMD_OPTIONS &
else
show_message "[4/7] File $mt5file is not installed. MT5 cannot be run."
</file context>
|
|
||
| ### MetaTrader 5 Command Line Options | ||
|
|
||
| You can pass command line options to MetaTrader 5 using the `MT5_CMD_OPTIONS` environment variable. This is useful for custom configurations, tester modes, or other MetaTrader command line parameters. |
There was a problem hiding this comment.
P2: Command-line options containing spaces or shell-special characters will not reach MetaTrader 5 as documented because MT5_CMD_OPTIONS is expanded unquoted by the startup script. Please document that only shell-word-safe values are supported or change the launcher to parse/pass options safely (for example, an array-based interface).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 188:
<comment>Command-line options containing spaces or shell-special characters will not reach MetaTrader 5 as documented because `MT5_CMD_OPTIONS` is expanded unquoted by the startup script. Please document that only shell-word-safe values are supported or change the launcher to parse/pass options safely (for example, an array-based interface).</comment>
<file context>
@@ -148,20 +180,65 @@ True
+
+### MetaTrader 5 Command Line Options
+
+You can pass command line options to MetaTrader 5 using the `MT5_CMD_OPTIONS` environment variable. This is useful for custom configurations, tester modes, or other MetaTrader command line parameters.
+
+Example using docker-compose:
</file context>
| You can pass command line options to MetaTrader 5 using the `MT5_CMD_OPTIONS` environment variable. This is useful for custom configurations, tester modes, or other MetaTrader command line parameters. | |
| You can pass shell-word-safe command line options to MetaTrader 5 using the `MT5_CMD_OPTIONS` environment variable. Options containing spaces or shell metacharacters are not supported by the current launcher. |
| image: gmag11/metatrader5_vnc | ||
| container_name: mt5 | ||
| volumes: | ||
| - mt5-config:/config |
There was a problem hiding this comment.
P3: The managed volume is named mt5-config (hyphen) here, but the README's Windows configuration specifies mt5_config (underscore) both as the mount and in the top-level volumes section. It works internally since the name is consistent, but it deviates from the documented convention and will confuse anyone cross-referencing the README or scripts that expect mt5_config. Consider renaming to mt5_config for consistency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docker-compose-windows.yaml, line 7:
<comment>The managed volume is named `mt5-config` (hyphen) here, but the README's Windows configuration specifies `mt5_config` (underscore) both as the mount and in the top-level volumes section. It works internally since the name is consistent, but it deviates from the documented convention and will confuse anyone cross-referencing the README or scripts that expect `mt5_config`. Consider renaming to `mt5_config` for consistency.</comment>
<file context>
@@ -0,0 +1,14 @@
+ image: gmag11/metatrader5_vnc
+ container_name: mt5
+ volumes:
+ - mt5-config:/config
+ ports:
+ - 3000:3000
</file context>
Summary by cubic
Adds support for custom MetaTrader 5 command-line options and improves Windows setup. Also updates the README with clearer usage and configuration examples.
MT5_CMD_OPTIONSto pass MT5 CLI flags when launching MT5.docker-compose-windows.yaml(uses a named volume and.env) and expand README with Windows guidance and config examples.Written for commit 2a3fe40. Summary will update on new commits.