Had another go at this and implementing the suggestions, it's a bit cleaner now, still have some TODOs to look over though but I wanted to get feedback on the save protection stuff and the general idea.

One thing that's nagging at me right now as well is that as it stands now, the script itself is in charge of creating the Sprite itself, and that seems like something we might want to avoid. Originally I left it like that because with adding isModified and having the script have to set the filename, we could leave it open for scripts to open multiple sprites from a single file, but if that's not behavior we intend to support, then I think creating the sprite behind the scenes and then passing it through an argument to the script so it can manipulate the pixels and then we can mark it as saved/etc is gonna be the way to go.

Trying to think if there's any use cases that this way of doing things might make impossible though.

Perhaps the new File Explorer -> File Associations menu added in #2606 is relevant? An API to add into there would be wonderful.

There is an assert failing when running ctest in debug mode and gui available:

    Start 61: keyboard_shortcuts_tests
1/2 Test #61: keyboard_shortcuts_tests .........Subprocess aborted***Exception:   0.17 sec
FT: Loading font ... FAIL
FT: Loading font ... FAIL
src/app/extensions.cpp:995: Assertion failed: !item.widget->parent()

    Start 62: app_tests
2/2 Test #62: app_tests ........................Subprocess aborted***Exception:   0.15 sec
[==========] Running 3 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 3 tests from App
[ RUN      ] App.JustClose
FT: Loading font ... FAIL
FT: Loading font ... FAIL
src/app/extensions.cpp:995: Assertion failed: !item.widget->parent()

About the API, I think we should be able to specify an extension argument to newFileFormat(), and avoid name and extensions if the user wish (probably extension or extensions could be used indifferently as a string or as a table, accepting both types and both argument names). The name by default could be the first extension specified.

About the API, I think we should be able to specify an extension argument to newFileFormat(), and avoid name and extensions if the user wish (probably extension or extensions could be used indifferently as a string or as a table, accepting both types and both argument names). The name by default could be the first extension specified.

So there'd be both an extension and an extensions argument? Seems a bit confusing.

I do agree that you could allow extensions to accept a string for the simple case of one extension, though. Making name optional also makes sense.

Fixed the test issues, although I couldn't reproduce the assert issue you were having @dacap, does this happen in a fresh profile? There could be an installed extension causing, maybe this check causes problems?

I've made name optional, and I changed away from unoredered_set for the extension list. Originally my idea was to use it just to get duplicate removal for free, but to make name optional I wanted to pick a stable name, so I might as well just use a standard vector and do a one-time check when registering the extension.

I also added the possibility to use a string or table in the extensions parameter and an extension parameter that takes a string (you can mix and match if... you wanna be weird about it)

Lastly, I removed the starting "." in some of the error messages regarding formats, so that they can be descriptive instead of just extension based, since this could potentially cause confusion (even with the aseprite format, if you were saving as .aseprite, the error message could mention .ase, or with .jpg/.jpeg. This could also help us eventually to have more descriptive names for formats which is helpful when you start introducing some extension-based ones.
Example:

Moved the arguments for onload and onsave to a single table and added clarification for formats in the error windows. (updated the extension for it)

As for id vs name, I think the latter is better since it's not really an identifier and it's not used elsewhere, it's just to show the user and it's optional.

I'm testing this right now, saw that the list of file formats is not sorted 👀

@dacap Fixed the sorting issue (did not realize we were using the FileFormatsManager order), and fixed the assert, the signals were not getting connected because AppMenus is initialized after extensions so we didn't have the instance available, changed that to a lambda instead of direct connection.

Fixed all the the stack issues (I just kinda forgot about that huh, maybe we could add a debug-only macro with this stack-checking helper and sprinkle it all over the place so that I don't make that mistake again 🙈).

I've added some more tests for the format registration function and for general completeness and especially failure paths.

maybe we could add a debug-only macro with this stack-checking helper and sprinkle it all over the place so that I don't make that mistake again 🙈).

Yeah, I was thinking the same (in the past I tried but avoid it because it's too much work for functions that return values). I'll add something after merging this PR.

Looks great! 🥂