-
Notifications
You must be signed in to change notification settings - Fork 5.2k
CAMEL-24397: camel-tui - Fix --record producing no cast file #25514
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |
| import java.util.Collections; | ||
| import java.util.Comparator; | ||
| import java.util.List; | ||
| import java.util.Locale; | ||
| import java.util.Optional; | ||
| import java.util.Queue; | ||
| import java.util.concurrent.CompletableFuture; | ||
|
|
@@ -82,6 +83,19 @@ public class CamelMonitor extends CamelCommand { | |
| private static final Logger LOG = System.getLogger(CamelMonitor.class.getName()); | ||
| private static final long DEFAULT_REFRESH_MS = 500; | ||
|
|
||
| /** | ||
| * The TamboUI system properties {@code --record} configures. They are process-wide, so the session that sets them | ||
| * clears them again on the way out; otherwise a later TUI backend created in the same JVM would still see recording | ||
| * enabled via {@code RecordingConfig.isEnabled()}. | ||
| */ | ||
| static final List<String> RECORD_PROPERTIES = List.of( | ||
| "tamboui.record", | ||
| "tamboui.record.config", | ||
| "tamboui.record.width", | ||
| "tamboui.record.height", | ||
| "tamboui.record.duration", | ||
| "tamboui.record.fps"); | ||
|
|
||
| // Compact tab bar (10 labels + 9 "|" dividers) needs 88 chars — that is the true minimum | ||
| private static final int MIN_WIDTH = 88; | ||
| private static final int MIN_HEIGHT = 24; | ||
|
|
@@ -101,6 +115,21 @@ public class CamelMonitor extends CamelCommand { | |
| arity = "0..1") | ||
| String record; | ||
|
|
||
| @CommandLine.Option(names = { "--record-size" }, | ||
| description = "Size of the recorded terminal for --record, as <cols>x<rows> (default: ${DEFAULT-VALUE})", | ||
| defaultValue = "200x50") | ||
| String recordSize = "200x50"; | ||
|
|
||
| @CommandLine.Option(names = { "--record-fps" }, | ||
| description = "Frames per second captured by --record (default: ${DEFAULT-VALUE})", | ||
| defaultValue = "10") | ||
| int recordFps = 10; | ||
|
|
||
| @CommandLine.Option(names = { "--record-duration" }, | ||
| description = "Maximum duration in milliseconds captured by --record (default: ${DEFAULT-VALUE})", | ||
| defaultValue = "120000") | ||
| int recordDuration = 120000; | ||
|
|
||
| @CommandLine.Option(names = { "--mcp" }, | ||
| description = "Enable embedded MCP server for AI agent access to the TUI") | ||
| boolean mcp; | ||
|
|
@@ -184,6 +213,68 @@ public CamelMonitor(CamelJBangMain main, ClassLoader classLoader) { | |
| this.classLoader = classLoader; | ||
| } | ||
|
|
||
| /** | ||
| * Parses a {@code --record-size} value such as {@code 160x44} into {@code [cols, rows]}. | ||
| */ | ||
| int[] parseRecordSize(String size) { | ||
|
ammachado marked this conversation as resolved.
|
||
| String[] parts = size == null ? new String[0] : size.toLowerCase(Locale.ROOT).split("x", -1); | ||
| if (parts.length == 2) { | ||
| try { | ||
| int cols = Integer.parseInt(parts[0].trim()); | ||
| int rows = Integer.parseInt(parts[1].trim()); | ||
| if (cols > 0 && rows > 0) { | ||
| return new int[] { cols, rows }; | ||
| } | ||
| } catch (NumberFormatException e) { | ||
| // fall through to the parameter error below | ||
| } | ||
| } | ||
| throw new CommandLine.ParameterException( | ||
| new CommandLine(this), | ||
| "Invalid value for option '--record-size': expected '<cols>x<rows>' with positive numbers, was '" | ||
| + size + "'"); | ||
| } | ||
|
|
||
| /** | ||
| * Hands the {@code --record*} options to TamboUI through the {@link #RECORD_PROPERTIES} system properties, which is | ||
| * the only way TamboUI accepts a recording configuration. | ||
| */ | ||
| void configureRecording() { | ||
| if (record == null) { | ||
| return; | ||
| } | ||
| if (web) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved — rejecting |
||
| // The properties below are process-wide, so every browser session spawned by TuiWebServer would be | ||
| // wrapped for recording too, all writing the same cast file. The two modes are also conceptually | ||
| // exclusive: --record drives a headless TUI from a tape rather than from a connected terminal. | ||
| throw new CommandLine.ParameterException( | ||
| new CommandLine(this), | ||
| "Option '--record' cannot be combined with '--web': recording replays a tape headlessly " | ||
| + "and would be inherited by every browser session"); | ||
| } | ||
| Path tapeFile = Path.of(record); | ||
| Path castFile = Path.of(record.replaceAll("\\.tape$", "") + ".cast"); | ||
| int[] size = parseRecordSize(recordSize); | ||
| System.setProperty("tamboui.record", castFile.toAbsolutePath().toString()); | ||
| System.setProperty("tamboui.record.config", tapeFile.toAbsolutePath().toString()); | ||
| System.setProperty("tamboui.record.width", String.valueOf(size[0])); | ||
| System.setProperty("tamboui.record.height", String.valueOf(size[1])); | ||
| System.setProperty("tamboui.record.duration", String.valueOf(recordDuration)); | ||
| System.setProperty("tamboui.record.fps", String.valueOf(recordFps)); | ||
| } | ||
|
|
||
| /** | ||
| * Undoes {@link #configureRecording()} at the end of the session that ran it. | ||
| * <p> | ||
| * The already-loaded {@code RecordingConfig} keeps its own copy, so the shutdown hook still writes the cast file; | ||
| * clearing only stops a TUI backend created later in the same JVM from being wrapped for recording again. | ||
| */ | ||
| void clearRecordingProperties() { | ||
| for (String key : RECORD_PROPERTIES) { | ||
| System.clearProperty(key); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public Integer doCall() throws Exception { | ||
| System.setProperty("java.awt.headless", "true"); | ||
|
|
@@ -209,16 +300,7 @@ public Integer doCall() throws Exception { | |
| } | ||
|
|
||
| // Configure TamboUI recording if --record is specified | ||
| if (record != null) { | ||
| Path tapeFile = Path.of(record); | ||
| Path castFile = Path.of(record.replaceAll("\\.tape$", "") + ".cast"); | ||
| System.setProperty("tamboui.record", castFile.toAbsolutePath().toString()); | ||
| System.setProperty("tamboui.record.config", tapeFile.toAbsolutePath().toString()); | ||
| System.setProperty("tamboui.record.width", "200"); | ||
| System.setProperty("tamboui.record.height", "50"); | ||
| System.setProperty("tamboui.record.duration", "120000"); | ||
| System.setProperty("tamboui.record.fps", "10"); | ||
| } | ||
| configureRecording(); | ||
|
|
||
| recordingManager.init(record != null); | ||
|
|
||
|
|
@@ -649,6 +731,10 @@ public void resetIntegrationTabState() { | |
| } | ||
| deleteMcpJson(mcpJsonFile); | ||
| this.runner = null; | ||
| if (record != null) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved — |
||
| // Only the session that set the properties clears them again | ||
| clearRecordingProperties(); | ||
| } | ||
| } | ||
| return 0; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,21 @@ public class TuiCommand extends CamelCommand { | |
| arity = "0..1") | ||
| String record; | ||
|
|
||
| @CommandLine.Option(names = { "--record-size" }, | ||
| description = "Size of the recorded terminal for --record, as <cols>x<rows> (default: ${DEFAULT-VALUE})", | ||
| defaultValue = "200x50") | ||
| String recordSize = "200x50"; | ||
|
|
||
| @CommandLine.Option(names = { "--record-fps" }, | ||
| description = "Frames per second captured by --record (default: ${DEFAULT-VALUE})", | ||
| defaultValue = "10") | ||
| int recordFps = 10; | ||
|
|
||
| @CommandLine.Option(names = { "--record-duration" }, | ||
| description = "Maximum duration in milliseconds captured by --record (default: ${DEFAULT-VALUE})", | ||
| defaultValue = "120000") | ||
| int recordDuration = 120000; | ||
|
|
||
| @CommandLine.Option(names = { "--theme" }, | ||
| description = "Color theme: dark or light (overrides persisted preference for this session)", | ||
| completionCandidates = ThemeModeCompletionCandidates.class) | ||
|
|
@@ -71,6 +86,17 @@ public TuiCommand(CamelJBangMain main, ClassLoader classLoader) { | |
|
|
||
| @Override | ||
| public Integer doCall() throws Exception { | ||
| CamelMonitor cmd = new CamelMonitor(getMain(), classLoader); | ||
| return new CommandLine(cmd).execute(buildArgs().toArray(String[]::new)); | ||
| } | ||
|
|
||
| /** | ||
| * Builds the {@link CamelMonitor} command line this command delegates to. | ||
| * <p> | ||
| * Every option declared here must be forwarded, otherwise the option is silently accepted and then ignored. Only | ||
| * non-default values are passed on, so the delegate keeps applying its own defaults. | ||
| */ | ||
| List<String> buildArgs() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved — |
||
| List<String> args = new ArrayList<>(); | ||
| if (name != null) { | ||
| args.add(name); | ||
|
|
@@ -97,11 +123,22 @@ public Integer doCall() throws Exception { | |
| args.add("--record"); | ||
| args.add(record); | ||
| } | ||
| if (!"200x50".equals(recordSize)) { | ||
| args.add("--record-size"); | ||
| args.add(recordSize); | ||
| } | ||
| if (recordFps != 10) { | ||
| args.add("--record-fps"); | ||
| args.add(String.valueOf(recordFps)); | ||
| } | ||
| if (recordDuration != 120000) { | ||
| args.add("--record-duration"); | ||
| args.add(String.valueOf(recordDuration)); | ||
| } | ||
| if (theme != null) { | ||
| args.add("--theme"); | ||
| args.add(theme); | ||
| } | ||
| CamelMonitor cmd = new CamelMonitor(getMain(), classLoader); | ||
| return new CommandLine(cmd).execute(args.toArray(String[]::new)); | ||
| return args; | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.