Skip to content

feat: support supplementary environment variables for hooks in App - #5916

Merged
mergify[bot] merged 7 commits into
modrinth:mainfrom
Slackow:main
Aug 4, 2026
Merged

feat: support supplementary environment variables for hooks in App#5916
mergify[bot] merged 7 commits into
modrinth:mainfrom
Slackow:main

Conversation

@Slackow

@Slackow Slackow commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Implementation of the following feature:
Closes #1238

This is a feature present on other popular launchers (e.g. MultiMC, PrismLauncher, ATLauncher), so it would be great to include here as well to support more advanced hooking.

There was a PR open for this a while ago that was closed for being stale:
#1254

The difference with this PR is that instead of only substituting the text in the command, the variables are also provided as part of the environment, and arbitrary environment variables can be expanded. (e.g. $HOME), this is more in line with other launcher's implementations, and it also applies to all 3 hooks.

This does make it larger in scope, so a more thorough review is probably required, especially as a first time contributor. Thank you.

@modrinth-bot

modrinth-bot commented Apr 26, 2026

Copy link
Copy Markdown
Member

Pull request changelog

App

Added

  • Added support for environment variables in pre-launch, wrapper, and post-launch hooks.

Changed

Deprecated

Removed

Fixed

Security

Website

Added

Changed

Deprecated

Removed

Fixed

Security

Hosting

Added

Changed

Deprecated

Removed

Fixed

Security

Comment thread apps/app-frontend/src/components/ui/settings/DefaultInstanceSettings.vue Outdated
@Slackow

Slackow commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

Should explain what each variable represents like the other launchers.

Added a bulleted list describing the variables:

global hook settings instance hook settings

@Slackow
Slackow requested a review from fetchfern May 4, 2026 15:45
@Slackow
Slackow force-pushed the main branch 5 times, most recently from 45c164d to 152e39a Compare May 11, 2026 23:36
@Slackow
Slackow force-pushed the main branch 2 times, most recently from 76ccafb to de0827c Compare May 20, 2026 02:41
@Prospector
Prospector enabled auto-merge July 29, 2026 20:54
@Prospector
Prospector disabled auto-merge August 4, 2026 21:28
@Prospector

Copy link
Copy Markdown
Member

@Mergifyio queue

@mergify

mergify Bot commented Aug 4, 2026

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 10 minutes 45 seconds in the queue, including 9 minutes 36 seconds running CI.

Required conditions to merge

@Prospector

Copy link
Copy Markdown
Member

@Mergifyio refresh

@mergify

mergify Bot commented Aug 4, 2026

Copy link
Copy Markdown

refresh

✅ Pull request refreshed

@Prospector
Prospector requested review from fetchfern and removed request for fetchfern August 4, 2026 22:06
@Prospector

Copy link
Copy Markdown
Member

@Mergifyio refresh

@mergify

mergify Bot commented Aug 4, 2026

Copy link
Copy Markdown

refresh

✅ Pull request refreshed

@mergify mergify Bot added the queued label Aug 4, 2026
@mergify
mergify Bot merged commit cc53d9a into modrinth:main Aug 4, 2026
7 checks passed
@mergify mergify Bot removed the queued label Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Support environment variables for the profile hooks

5 participants