Skip to content
This repository was archived by the owner on Jan 9, 2026. It is now read-only.

don't just delete the m3u file for multi-part roms#58

Merged
gantoine merged 3 commits intorommapp:mainfrom
Percentnineteen:multi_part_delete
Jun 7, 2025
Merged

don't just delete the m3u file for multi-part roms#58
gantoine merged 3 commits intorommapp:mainfrom
Percentnineteen:multi_part_delete

Conversation

@Percentnineteen
Copy link
Contributor

@Percentnineteen Percentnineteen commented Jun 6, 2025

Description
multi-disk roms only deleted the .m3u which doesn't claw back much disk space. Logic is added to traverse the .m3u file and delete the files that it points at

Checklist
Please check all that apply.

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR

Screenshots

@gantoine
Copy link
Member

gantoine commented Jun 6, 2025

I'm going to test this in a bit since I don't think the new behaviour is doing what we're expecting it to...

@gantoine gantoine requested review from Copilot and gantoine June 6, 2025 21:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the ROM removal flow by deleting all files referenced in a multi-part ROM’s .m3u manifest instead of only removing the manifest itself.

  • Replaces the inline os.remove in the contextual menu with a call to a new helper.
  • Introduces _remove_rom_files to traverse .m3u lists and delete each referenced file.
  • Preserves single-file ROM removal behavior for non-multi ROMs.
Comments suppressed due to low confidence (3)

RomM/romm.py:874

  • Missing docstring for _remove_rom_files. Add a brief description of the method’s purpose, parameters, and behavior.
def _remove_rom_files(self, rom):

RomM/romm.py:880

  • [nitpick] The variable f is too generic. Consider renaming it to m3u_file or file_handle to clarify its role.
with open(rom_list_path, "r") as f:

RomM/romm.py:877

  • Add unit tests for the multi-part ROM deletion path to ensure that all files listed in the .m3u are correctly removed and edge cases (empty lines, missing files) are handled.
if rom.multi:

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Percentnineteen
Copy link
Contributor Author

I'm going to test this in a bit since I don't think the new behaviour is doing what we're expecting it to...

It worked correctly for me when I tested on both a psx game with multiple chd files in the .hidden directory as well as when I tested on a nds game which is by nature a single file.

Is there a particular concern you had?

@gantoine
Copy link
Member

gantoine commented Jun 6, 2025

Is there a particular concern you had?

No concern, I'm just having trouble visualizing the new logic. Once I run things on my device with some logs it should all make sense!

Copy link
Member

@gantoine gantoine left a comment

Choose a reason for hiding this comment

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

Ah I get it now! I would have deleted matching files in the .hidden folder but I think your approach is more robust. 💪🏼

@gantoine gantoine merged commit 1788dad into rommapp:main Jun 7, 2025
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants