mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
Merge branch 'agentydragon-22-message-rendering-layout-options' into agentydragon
This commit is contained in:
@@ -1,9 +1,9 @@
|
||||
+++
|
||||
id = "22"
|
||||
title = "Message Separation and Sender-Content Layout Options"
|
||||
status = "Not started"
|
||||
status = "Done"
|
||||
dependencies = "" # No prerequisites
|
||||
last_updated = "2025-06-25T01:40:09.600000"
|
||||
last_updated = "2025-06-25T11:05:55.000000"
|
||||
+++
|
||||
|
||||
## Summary
|
||||
@@ -24,6 +24,7 @@ Provide users with flexibility in how chat messages are visually separated and h
|
||||
- Add unit tests to verify the layout produces the correct sequence of lines.
|
||||
|
||||
## Implementation
|
||||
### Plan
|
||||
|
||||
**How it was implemented**
|
||||
- Extend the chat UI renderer to read `message_spacing` from config.
|
||||
|
||||
@@ -37,12 +37,15 @@ def resolve_slug(input_id: str) -> str:
|
||||
@click.option('-i', '--interactive', is_flag=True,
|
||||
help='Run agent in interactive mode (no exec); implies --agent.')
|
||||
@click.option('-s', '--shell', 'shell_mode', is_flag=True,
|
||||
help='Launch an interactive Codex shell (skip auto-commit); implies --agent.')
|
||||
help='Launch an interactive Codex shell (skip exec and auto-commit); implies --agent and --interactive.')
|
||||
@click.option('--skip-presubmit', is_flag=True,
|
||||
help='Skip the initial presubmit pre-commit checks when creating a new worktree.')
|
||||
@click.argument('task_inputs', nargs=-1, required=True)
|
||||
def main(agent, tmux_mode, interactive, shell_mode, skip_presubmit, task_inputs):
|
||||
"""Create/reuse a task worktree and optionally launch a Dev agent or tmux session."""
|
||||
# shell mode implies interactive (skip exec within the worktree)
|
||||
if shell_mode:
|
||||
interactive = True
|
||||
if interactive or shell_mode:
|
||||
agent = True
|
||||
|
||||
|
||||
@@ -32,6 +32,11 @@ def main(task_input):
|
||||
msg_file = Path(subprocess.check_output(['mktemp']).decode().strip())
|
||||
try:
|
||||
os.chdir(wt)
|
||||
# Abort early if no pending changes in this worktree
|
||||
status_out = subprocess.check_output(['git', 'status', '--porcelain'], text=True).strip()
|
||||
if not status_out:
|
||||
click.echo(f"No changes detected in worktree for '{slug}'; nothing to commit.", err=True)
|
||||
sys.exit(0)
|
||||
cmd = ['codex', '--full-auto', 'exec', '--output-last-message', str(msg_file)]
|
||||
click.echo(f"Running: {' '.join(cmd)}")
|
||||
prompt_content = prompt_file.read_text(encoding='utf-8')
|
||||
|
||||
@@ -426,5 +426,12 @@ composer_max_rows = 10 # defaults to `10`
|
||||
|
||||
# Command used to launch an external editor for composing the chat prompt.
|
||||
# Defaults to `$VISUAL`, then `$EDITOR`, falling back to `nvim`.
|
||||
|
||||
editor = "${VISUAL:-${EDITOR:-nvim}}"
|
||||
|
||||
# Insert a blank line between messages for visual separation.
|
||||
message_spacing = false # defaults to `false`
|
||||
|
||||
# Render the sender label on its own line above the message content.
|
||||
sender_break_line = false # defaults to `false`
|
||||
```
|
||||
|
||||
@@ -98,6 +98,14 @@ pub struct Tui {
|
||||
#[serde(default)]
|
||||
pub header_compact: bool,
|
||||
|
||||
/// When `true`, insert a blank line between messages for visual separation.
|
||||
#[serde(default)]
|
||||
pub message_spacing: bool,
|
||||
|
||||
/// When `true`, render the sender label on its own line above the message content.
|
||||
#[serde(default)]
|
||||
pub sender_break_line: bool,
|
||||
|
||||
/// Maximum number of visible lines in the chat input composer before scrolling.
|
||||
/// The composer will expand up to this many lines; additional content will enable
|
||||
/// an internal scrollbar.
|
||||
@@ -135,6 +143,8 @@ impl Default for Tui {
|
||||
disable_mouse_capture: Default::default(),
|
||||
markdown_compact: Default::default(),
|
||||
header_compact: Default::default(),
|
||||
message_spacing: Default::default(),
|
||||
sender_break_line: Default::default(),
|
||||
composer_max_rows: default_composer_max_rows(),
|
||||
editor: default_editor(),
|
||||
require_double_ctrl_d: false,
|
||||
|
||||
@@ -228,8 +228,34 @@ pub(crate) fn new_user_prompt(config: &Config, message: String) -> Self {
|
||||
"user".to_string(),
|
||||
Style::default().fg(Color::Cyan).add_modifier(Modifier::BOLD),
|
||||
);
|
||||
let mut lines = render_header_body(config, label, body);
|
||||
lines.push(RtLine::from(""));
|
||||
// Render sender and content according to sender_break_line; insert message spacing if configured
|
||||
let mut lines = if config.tui.sender_break_line {
|
||||
let mut l = Vec::new();
|
||||
// label on its own line
|
||||
l.push(RtLine::from(vec![label.clone()]));
|
||||
// then message body lines
|
||||
l.extend(body.clone());
|
||||
l
|
||||
} else {
|
||||
// combine sender label and first line of body, indenting subsequent lines
|
||||
let mut l = Vec::new();
|
||||
if let Some(first) = body.get(0) {
|
||||
let mut spans = vec![label.clone(), RtSpan::raw(" ".to_string())];
|
||||
spans.extend(first.spans.clone());
|
||||
l.push(RtLine::from(spans).style(first.style));
|
||||
let indent = " ".to_string();
|
||||
for ln in body.iter().skip(1) {
|
||||
let text: String = ln.spans.iter().map(|s| s.content.clone()).collect();
|
||||
l.push(RtLine::from(indent.clone() + &text));
|
||||
}
|
||||
} else {
|
||||
l.push(RtLine::from(vec![label.clone()]));
|
||||
}
|
||||
l
|
||||
};
|
||||
if config.tui.message_spacing {
|
||||
lines.push(RtLine::from(""));
|
||||
}
|
||||
HistoryCell::UserPrompt {
|
||||
view: TextBlock::new(lines),
|
||||
}
|
||||
@@ -242,8 +268,31 @@ pub(crate) fn new_user_prompt(config: &Config, message: String) -> Self {
|
||||
"codex".to_string(),
|
||||
Style::default().fg(Color::Magenta).add_modifier(Modifier::BOLD),
|
||||
);
|
||||
let mut lines = render_header_body(config, label, md_lines);
|
||||
lines.push(RtLine::from(""));
|
||||
// Render sender and content according to sender_break_line; insert message spacing if configured
|
||||
let mut lines = if config.tui.sender_break_line {
|
||||
let mut l = Vec::new();
|
||||
l.push(RtLine::from(vec![label.clone()]));
|
||||
l.extend(md_lines.clone());
|
||||
l
|
||||
} else {
|
||||
let mut l = Vec::new();
|
||||
if let Some(first) = md_lines.get(0) {
|
||||
let mut spans = vec![label.clone(), RtSpan::raw(" ".to_string())];
|
||||
spans.extend(first.spans.clone());
|
||||
l.push(RtLine::from(spans).style(first.style));
|
||||
let indent = " ".to_string();
|
||||
for ln in md_lines.iter().skip(1) {
|
||||
let text: String = ln.spans.iter().map(|s| s.content.clone()).collect();
|
||||
l.push(RtLine::from(indent.clone() + &text));
|
||||
}
|
||||
} else {
|
||||
l.push(RtLine::from(vec![label.clone()]));
|
||||
}
|
||||
l
|
||||
};
|
||||
if config.tui.message_spacing {
|
||||
lines.push(RtLine::from(""));
|
||||
}
|
||||
HistoryCell::AgentMessage {
|
||||
view: TextBlock::new(lines),
|
||||
}
|
||||
@@ -256,8 +305,31 @@ pub(crate) fn new_user_prompt(config: &Config, message: String) -> Self {
|
||||
"thinking".to_string(),
|
||||
Style::default().fg(Color::Magenta).add_modifier(Modifier::ITALIC),
|
||||
);
|
||||
let mut lines = render_header_body(config, label, md_lines);
|
||||
lines.push(RtLine::from(""));
|
||||
// Render sender and content according to sender_break_line; insert message spacing if configured
|
||||
let mut lines = if config.tui.sender_break_line {
|
||||
let mut l = Vec::new();
|
||||
l.push(RtLine::from(vec![label.clone()]));
|
||||
l.extend(md_lines.clone());
|
||||
l
|
||||
} else {
|
||||
let mut l = Vec::new();
|
||||
if let Some(first) = md_lines.get(0) {
|
||||
let mut spans = vec![label.clone(), RtSpan::raw(" ".to_string())];
|
||||
spans.extend(first.spans.clone());
|
||||
l.push(RtLine::from(spans).style(first.style));
|
||||
let indent = " ".to_string();
|
||||
for ln in md_lines.iter().skip(1) {
|
||||
let text: String = ln.spans.iter().map(|s| s.content.clone()).collect();
|
||||
l.push(RtLine::from(indent.clone() + &text));
|
||||
}
|
||||
} else {
|
||||
l.push(RtLine::from(vec![label.clone()]));
|
||||
}
|
||||
l
|
||||
};
|
||||
if config.tui.message_spacing {
|
||||
lines.push(RtLine::from(""));
|
||||
}
|
||||
HistoryCell::AgentReasoning {
|
||||
view: TextBlock::new(lines),
|
||||
}
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
use tempfile::TempDir;
|
||||
use codex_core::config::{Config, ConfigToml, ConfigOverrides};
|
||||
use codex_tui::history_cell::HistoryCell;
|
||||
|
||||
/// Extract plain string content of each line for comparison.
|
||||
fn lines_from_userprompt(view: &codex_tui::text_block::TextBlock) -> Vec<String> {
|
||||
view.lines
|
||||
.iter()
|
||||
.map(|line| line.spans.iter().map(|s| s.content.clone()).collect())
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_user_message_layout_combinations() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let mut config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides::default(),
|
||||
tmp.path().to_path_buf(),
|
||||
)
|
||||
.unwrap();
|
||||
let message = "first line\nsecond line".to_string();
|
||||
for &sender_break in &[false, true] {
|
||||
for &message_spacing in &[false, true] {
|
||||
config.tui.sender_break_line = sender_break;
|
||||
config.tui.message_spacing = message_spacing;
|
||||
let cell = HistoryCell::new_user_prompt(&config, message.clone());
|
||||
let view = match cell {
|
||||
HistoryCell::UserPrompt { view } => view,
|
||||
_ => panic!("expected UserPrompt variant"),
|
||||
};
|
||||
let got = lines_from_userprompt(&view);
|
||||
let mut expected = Vec::new();
|
||||
if sender_break {
|
||||
expected.push("user".to_string());
|
||||
expected.push("first line".to_string());
|
||||
expected.push("second line".to_string());
|
||||
} else {
|
||||
expected.push("user first line".to_string());
|
||||
expected.push(" second line".to_string());
|
||||
}
|
||||
if message_spacing {
|
||||
expected.push(String::new());
|
||||
}
|
||||
assert_eq!(got, expected,
|
||||
"Layout mismatch for sender_break_line={}, message_spacing={}",
|
||||
sender_break, message_spacing);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_agent_message_layout_combinations() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let mut config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides::default(),
|
||||
tmp.path().to_path_buf(),
|
||||
)
|
||||
.unwrap();
|
||||
let message = "first line\nsecond line".to_string();
|
||||
for &sender_break in &[false, true] {
|
||||
for &message_spacing in &[false, true] {
|
||||
config.tui.sender_break_line = sender_break;
|
||||
config.tui.message_spacing = message_spacing;
|
||||
let cell = HistoryCell::new_agent_message(&config, message.clone());
|
||||
let view = match cell {
|
||||
HistoryCell::AgentMessage { view } => view,
|
||||
_ => panic!("expected AgentMessage variant"),
|
||||
};
|
||||
let got = lines_from_userprompt(&view);
|
||||
let mut expected = Vec::new();
|
||||
if sender_break {
|
||||
expected.push("codex".to_string());
|
||||
expected.push("first line".to_string());
|
||||
expected.push("second line".to_string());
|
||||
} else {
|
||||
expected.push("codex first line".to_string());
|
||||
expected.push(" second line".to_string());
|
||||
}
|
||||
if message_spacing {
|
||||
expected.push(String::new());
|
||||
}
|
||||
assert_eq!(got, expected,
|
||||
"Agent layout mismatch for sender_break_line={}, message_spacing={}",
|
||||
sender_break, message_spacing);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_agent_reasoning_layout_combinations() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let mut config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides::default(),
|
||||
tmp.path().to_path_buf(),
|
||||
)
|
||||
.unwrap();
|
||||
let message = "first line\nsecond line".to_string();
|
||||
for &sender_break in &[false, true] {
|
||||
for &message_spacing in &[false, true] {
|
||||
config.tui.sender_break_line = sender_break;
|
||||
config.tui.message_spacing = message_spacing;
|
||||
let cell = HistoryCell::new_agent_reasoning(&config, message.clone());
|
||||
let view = match cell {
|
||||
HistoryCell::AgentReasoning { view } => view,
|
||||
_ => panic!("expected AgentReasoning variant"),
|
||||
};
|
||||
let got = lines_from_userprompt(&view);
|
||||
let mut expected = Vec::new();
|
||||
if sender_break {
|
||||
expected.push("thinking".to_string());
|
||||
expected.push("first line".to_string());
|
||||
expected.push("second line".to_string());
|
||||
} else {
|
||||
expected.push("thinking first line".to_string());
|
||||
expected.push(" second line".to_string());
|
||||
}
|
||||
if message_spacing {
|
||||
expected.push(String::new());
|
||||
}
|
||||
assert_eq!(got, expected,
|
||||
"Reasoning layout mismatch for sender_break_line={}, message_spacing={}",
|
||||
sender_break, message_spacing);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user