Skip to content

losslesscut: Fix file associations & general manifest updates#17889

Open
NECOtype wants to merge 4 commits into
ScoopInstaller:masterfrom
NECOtype:losslesscut-file-associations
Open

losslesscut: Fix file associations & general manifest updates#17889
NECOtype wants to merge 4 commits into
ScoopInstaller:masterfrom
NECOtype:losslesscut-file-associations

Conversation

@NECOtype

Copy link
Copy Markdown
Contributor

Closes #16723

The original .reg files werent working anymore.

This PR

  • fixes .reg files
  • updates post_install and uninstaller scripts
  • updates the note for correct way of importing .reg files
  • updates homepage url and checkver for github
  • reorder, puts post_install before bin
  • Use conventional PR title: <manifest-name[@version]|chore>: <general summary of the pull request>
  • I have read the Contributing Guide

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7dfbcc96-5090-4fae-8d5f-1dd85a82f4c9

📥 Commits

Reviewing files that changed from the base of the PR and between ccc751c and 5ba8930.

📒 Files selected for processing (1)
  • scripts/losslesscut/install-associations.reg

📝 Walkthrough

Walkthrough

This PR refactors LosslessCut's Windows file association setup by replacing the old Applications\LosslessCut.exe registry scheme with a new LosslessCut-based registration structure. The install-associations.reg and uninstall-associations.reg registry scripts are updated to define and remove the new registry keys. The Scoop manifest's PowerShell post_install logic is reworked to load registry templates from the bucket scripts directory, perform placeholder substitution for the install path, and write the generated .reg files. The uninstaller.script is simplified, checkver is changed to an explicit GitHub URL object, and the homepage metadata is updated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ScoopInstaller/Extras#17728: Both PRs rework Scoop manifest PowerShell install/uninstall flow to generate and import Windows .reg registry association files while updating the associated .reg scripts.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly summarizes the main changes: fixing file associations and updating the manifest configuration.
Description check ✅ Passed The PR description follows the template with linked issue (#16723), checked compliance checkboxes, and detailed bullet points explaining all changes made.
Linked Issues check ✅ Passed The PR successfully implements the core requirement from #16723: adding Windows registry entries to create a context menu shortcut for LosslessCut file associations with media files.
Out of Scope Changes check ✅ Passed All changes align with the stated objectives: fixing .reg files, updating scripts, correcting homepage URL, improving checkver configuration, and reordering manifest fields.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@NECOtype

Copy link
Copy Markdown
Contributor Author

/verify

@github-actions

Copy link
Copy Markdown
Contributor

All changes look good.

Wait for review from human collaborators.

losslesscut

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

Check the full log for details.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
bucket/losslesscut.json (1)

16-45: Please run the manifest validation flow locally before merge.

Recommended commands for this manifest (losslesscut):

scoop config debug true
scoop config gh_token <your-github-token>  # read-only token

.\bin\checkver.ps1 -App losslesscut -f
.\bin\formatjson.ps1 -App losslesscut
scoop install bucket\losslesscut.json -a 64bit

References:

As per coding guidelines: “Provide clear instructions for testing the manifest locally before submission” and include the official contribution guide/wiki links.

🤖 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 `@bucket/losslesscut.json` around lines 16 - 45, The manifest for losslesscut
(bucket/losslesscut.json) must be validated and tested locally before merging:
run the checkver and format scripts and install the manifest in 64-bit mode to
ensure the post_install script, checkver (github), and autoupdate URL work
correctly—specifically run the equivalent of .\bin\checkver.ps1 -App losslesscut
-f, .\bin\formatjson.ps1 -App losslesscut, and attempt scoop install of the
manifest (64bit) to exercise post_install reg file generation and the
uninstaller script; update the PR with confirmation that these commands passed
and include the Contribution Guide and Wiki links per guidelines.
🤖 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 `@scripts/losslesscut/install-associations.reg`:
- Line 4: The InstallLocation registry entry currently writes literal embedded
quotes (the line containing "InstallLocation"="\"$losslesscut\"" ), causing the
value to include double-quote characters; change that entry so it writes the raw
$losslesscut path without surrounding/escaped quotes, i.e. remove the embedded
escaped quotes in the InstallLocation value assignment so the registry stores
the plain path.

---

Nitpick comments:
In `@bucket/losslesscut.json`:
- Around line 16-45: The manifest for losslesscut (bucket/losslesscut.json) must
be validated and tested locally before merging: run the checkver and format
scripts and install the manifest in 64-bit mode to ensure the post_install
script, checkver (github), and autoupdate URL work correctly—specifically run
the equivalent of .\bin\checkver.ps1 -App losslesscut -f, .\bin\formatjson.ps1
-App losslesscut, and attempt scoop install of the manifest (64bit) to exercise
post_install reg file generation and the uninstaller script; update the PR with
confirmation that these commands passed and include the Contribution Guide and
Wiki links per guidelines.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 29c79e48-6618-42fd-80ea-5cddc166b684

📥 Commits

Reviewing files that changed from the base of the PR and between fe5e654 and ccc751c.

📒 Files selected for processing (3)
  • bucket/losslesscut.json
  • scripts/losslesscut/install-associations.reg
  • scripts/losslesscut/uninstall-associations.reg

Comment thread scripts/losslesscut/install-associations.reg Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@NECOtype

Copy link
Copy Markdown
Contributor Author

/verify

@github-actions

Copy link
Copy Markdown
Contributor

All changes look good.

Wait for review from human collaborators.

losslesscut

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

Check the full log for details.

arvdk added a commit to arvdk/Extras that referenced this pull request Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] LosslessCut - Add context menu shortcut

1 participant