Skip to content

Add optional maximum title length setting - #32

Open
shaunmower wants to merge 9 commits into
Poisonite:masterfrom
shaunmower:feat/max-title-duration
Open

shaunmower wants to merge 9 commits into
Poisonite:masterfrom
shaunmower:feat/max-title-duration

Conversation

@shaunmower

Copy link
Copy Markdown

Description

Adds an optional ripping.max_title_length_minutes setting that skips titles longer than N minutes. It is useful for TV discs, where a "play all" title is longer than any episode.

  • With rip_all_titles: true, every title at or below the limit is ripped. Each one is a separate makemkvcon call, run one after another.
  • With rip_all_titles: false, the longest title at or below the limit is ripped.
  • If no title on a disc is within the limit, the disc is logged and skipped.
  • If the setting is left out, or is not a positive number, nothing changes.
ripping:
  rip_all_titles: true
  max_title_length_minutes: 30

The default config.yaml includes the setting commented out (# max_title_length_minutes: 60), so the limit is off unless you turn it on.

The setting is also on the web UI config page as a number field. If you leave the field empty and save, the line is commented out in config.yaml. If you enter a value later, the line is uncommented again.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🧪 Tests

Testing

Unit tests added for:

  • AppConfig.maxTitleLengthMinutes
  • getFileNumbers() with rip-all on and off
  • getDiscFileInfo() returning fileNumbers when a limit is set
  • getCompleteDiscInfo() dropping skipped discs
  • ripSingleDisc() running one command per selected title

There is no test file for api.routes.js. I checked the YAML update logic separately by setting a value, clearing it and setting it again.

npx vitest run: 400 of 401 tests pass. The failing test is native-optical-drive.test.js > should validate drive letter parameter, which also fails on master without this change.

General Testing

  • Tested with DVD discs
  • Tested main ripping functionality (npm start)

Web UI Testing (if applicable)

  • Tested web interface (npm run web)
  • Tested configuration editing through web UI

Test Configuration:

  • OS: Windows 11
  • Node.js version: v24.19.0
  • MakeMKV version: v2.0.0
  • Administrator privileges: No

Documentation

  • Updated README.md if needed
  • Added/updated JSDoc comments for new code
  • Updated YAML configuration documentation if needed
  • Updated CHANGELOG.md
  • Updated web UI documentation if applicable

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • My changes generate no new warnings or errors

Additional Context

When several titles are ripped from one disc, their output is joined before the "Copy complete" check. As a result, the disc counts as successful if any one title reports "Copy complete". I can change this to check each title separately if you prefer.

This PR and #31 both add an [Unreleased] section to CHANGELOG.md, so whichever is merged second will have a small, easy conflict to resolve.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant