Headless CLI

[CoffeeFlux]

No description provided.

[arch1t3cht]

Thanks for cleaning this up and finishing this!

The rebase of my old cli branch looks good except for a couple of lines that cause compiler warnings now that the warning level was raised, see the following patches that can be squashed onto e97b25e and 4d738d5:

diff --git a/libaegisub/include/libaegisub/background_runner.h b/libaegisub/include/libaegisub/background_runner.h
index 743f5e643..63c6740fe 100644
--- a/libaegisub/include/libaegisub/background_runner.h
+++ b/libaegisub/include/libaegisub/background_runner.h
@@ -55,7 +55,7 @@ namespace agi {
 		virtual bool IsCancelled()=0;
 	};
 
-	/// @class DummyProgressSink
+	/// @class CLIProgressSink
 	/// @brief A progress sink that doesn't do anything with its progress updates except print logs
 	class CLIProgressSink : public ProgressSink {
 		std::ostream &stream;
@@ -64,11 +64,11 @@ namespace agi {
 		CLIProgressSink(std::ostream &stream = std::cout) : stream(stream) {};
 
 		void SetIndeterminate() {}
-		void SetTitle(std::string const& title) {}
-		void SetMessage(std::string const& title) {}
-		void SetProgress(int64_t cur, int64_t max) {}
+		void SetTitle(std::string const&) {}
+		void SetMessage(std::string const&) {}
+		void SetProgress(int64_t, int64_t) {}
 		void Log(std::string const& str) { stream << str; }
-		void SetStayOpen(bool stayopen) {}
+		void SetStayOpen(bool) {}
 		bool IsCancelled() { return false; }
 	};

diff --git a/src/auto4_lua_dialog.cpp b/src/auto4_lua_dialog.cpp
index 05d64fef4..98621449a 100644
--- a/src/auto4_lua_dialog.cpp
+++ b/src/auto4_lua_dialog.cpp
@@ -459,7 +459,7 @@ namespace Automation4 {
 			buttons.emplace_back(wxID_CANCEL, "");
 		}
 
-		for (int i = 0; i < buttons.size(); i++) {
+		for (int i = 0; i < std::ssize(buttons); i++) {
 			LOG_D("automation/lua/dialog") << "created button: " << buttons[i].second << " (" << i << ")";
 		}
 	}
@@ -541,7 +541,7 @@ namespace Automation4 {
 	}
 
 	void LuaDialog::PushButton(int button) {
-		if (button != -1 && (button < 0 || button >= buttons.size())) {
+		if (button != -1 && (button < 0 || button >= std::ssize(buttons))) {
 			LOG_E("agi/auto4_lua_dialog") << "Button " << button << " not in range; defaulting to cancel";
 			button = -1;
 		}

Almost all of the further additions also look good to me. The dispatch queue is pretty much what I was also planning to do back when I was working on this branch, and I definitely also agree with using CLI11 over boost's program_options (which I just copied from myaa's aegisub-cli back then).

(Adding cli11 to the --skip-subprojects arg in osx-bundle.sh should be enough to fix the Mac CI).

Apart from the review comments, I mainly just have a few thoughts on what else is eventually needed for CLI support:

• For full parity with aegisub-cli, the resolution resampler would also need to be exposed to the CLI. Though then again resolution resampling is becoming less and less relevant now that LayoutRes is available everywhere so maybe this isn't even needed any more?

• Back when working on this, I was considering making an actual aegisub-cli wrapper program that just passes its arguments on to aegisub --cli (on Linux/Mac this can be a simple symlink that branches on argv[0], on Windows the aegisub.com wrapper can be copied to aegisub-cli.com and also branch on argv[0]).

• The main use case for headless CLI at the moment are end-to-end tests for automation scripts, but of course, once this is released, it will get used by other tooling like muxtools and SubKt. So we'd need to try and get the CLI API right the first time so we don't need to do compat breaks in the future. Not that I have any concrete issues with the current API, but it's at least something to think about.

• My biggest worry is that Aegisub still loads and writes a bunch of global configuration, even in CLI mode, and I'm still not really sure how to best reconcile this:

  • On the one hand, with E2E tests or as part of some muxing pipeline, it would be best for aegisub-cli to be as self-contained and reproducible as possible
  • In particular there is probably no reason why aegisub-cli should write to the user's config/hotkeys/MRU/autosaves/etc.
  • On the other hand, users may want aegisub-cli to load their global automation scripts. Also, even if there was a way to set a different automation autoload/include directory for the CLI, there would need to be some easy way to install an automation script with all of its dependencies into that directory.
  • In some setups (portable Windows installs) it is simply impossible to load Aegisub's built-in Lua libraries without also loading (i.e. adding to the include path) user-installed libraries.
  • At the moment, DependencyControl scripts will probably still try to automatically update themselves when called in CLI mode, which is probably not a good idea? But on the other hand it would be nice if there was a way to explicitly request to install or update scripts from the CLI. There's probably no way to sort this out without DependencyControl (and hence maybe scripts in general) being explicitly aware of whether it's running in CLI mode or not, and where the automation directory is.

  The solution to all of this is probably some collection of command-line options to specify how Aegisub should interact with its global config, but I'm not yet sure what they should be. (Some prior art here is mpv with --no-config and --player-operation-mode. We should probably also check some other GUI applications that offer CLI modes, like e.g. Inkscape.). Maybe something like:

  • CLI mode does not load Aegisub's config.json by default (if necessary it can load a builtin config overlay for CLI mode that e.g. disables autosaving?) but a config.json can be passed on the command line (e.g. --config-string="<explicit JSON>" or --config-file=config.json or --config-user to load the user's config, if there is even a use-case for the latter)
    • Will need to check which config options even affect CLI mode. At the very least the audio/video provider and automation path options do.
  • By default, both ?data and ?user are added to the automation include path: This is out of necessity since ?data needs to be added for aegisub-cli to be in any way useful, and ?user is sometimes equal to ?data, but it would be nice to have some better solution here. But the path can be adjusted if needed - either via --config or even through some explicit command-line option.
  • Like aegisub-cli already does, no global automation scripts are loaded by default; scripts to load need to be explicitly passed on the command-line. But it'd be nice to also be able to pass a directory of scripts or, if a use-case comes up, to load Aegisub's global autoload directories.
  • How to handle a) DependencyControl's updates and b) the config files of scripts? DependencyControl probably shouldn't auto-update in headless mode unless explicitly requested to, and scripts maybe shouldn't write their own config/state files unless explicitly requested to, but then the question is how to explicitly request these. The simple solution for now would be to expose if Aegisub is running in CLI mode or not to Lua, but I'm not even sure if that is the best solution. The longer-term solution might be to integrate the config handling of scripts more directly into Aegisub's built-in API, which I wanted to do eventually anyway.

  But I'm still very unsure about much of this.

• The long-term plan should probably be to separate Aegisub's GUI front-end more and more from the subtitle/video/audio/automation loading (finishing the libaegisub split) and processing to make all of the CLI code paths less invasive, but that will need much bigger refactors.

[arch1t3cht]

Do they actually? AFAICT they explicitly do not dispatch to Main in CLI mode.

[arch1t3cht]

Maybe it's better to make Project::LoadSubtitles return a status instead? Project::DoLoadSubtitles already returns a bool so LoadSubtitles would just need to pass that on.

[arch1t3cht]

As far as I can tell it should be fairly simple to manually implement WinMain on Windows and forward the arguments to wxEntry? I can try this out later.

Though I guess the bigger issue there is that windows-subsystem programs can't send their stdout to the console. For that, my plan was to just steal mpv's solution, i.e. making an aegisub.com wrapper that calls aegisub.exe and redirects stdin/out.