Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 24 additions & 21 deletions crates/jp_cli/src/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,18 @@ pub(crate) enum WorkspaceRequirement {
/// plugin child cwd, path parsing — simply do not run.
None,

/// Resolve and validate a target root, without loading the conversation
/// The workspace is the command's *subject* rather than its context.
///
/// `jp w show <id>` reports on a workspace instead of running inside one,
/// and `jp w use <id>` records a selection for later runs to resolve.
/// Both resolve their own [`WorkspaceTarget`] against the pre-workspace
/// [`TargetEnv`], from outside every workspace and possibly to no workspace
/// at all, so selecting a root on their behalf would answer a question they
/// have not asked.
Subject,

/// Resolve a root, construct the [`Workspace`], and load the conversation
/// index.
Resolve,

/// Resolve, construct the [`Workspace`], and load the conversation index.
Load,
}

Expand All @@ -74,8 +81,7 @@ pub(crate) enum RootSource {
/// w use`).
SessionActive,

/// The interactive picker fallback, recorded as the session's new active
/// workspace.
/// The interactive picker fallback, resolving this run only.
Picker,
}

Expand Down Expand Up @@ -191,10 +197,14 @@ impl ExecutionContext {
/// 5. Else the session-active workspace, while the workspace still has a live
/// checkout — recovering through surviving checkouts when the recorded one
/// is gone.
/// 6. Else the picker, recorded as the new session-active workspace.
/// 6. Else the picker, resolving this run only.
///
/// Non-interactive runs ignore the session layer entirely (steps 2–3 and
/// 5–6), so scripts never depend on hidden per-session state.
///
/// Resolution may *repair* the session's recorded selection when its checkout
/// is gone, but never creates one: attaching a workspace to a session is `jp w
/// use`'s job, or the `C` / `A` answers to the conflict prompt.
pub(crate) fn resolve(
target: Option<&WorkspaceTarget>,
session: Option<&Session>,
Expand Down Expand Up @@ -261,7 +271,7 @@ fn source_for(target: &WorkspaceTarget) -> RootSource {
WorkspaceTarget::Session
| WorkspaceTarget::SessionPicker
| WorkspaceTarget::Picker
| WorkspaceTarget::Latest
| WorkspaceTarget::Recent
| WorkspaceTarget::Fuzzy(_) => RootSource::CliSelector,
WorkspaceTarget::Cwd | WorkspaceTarget::Help => {
unreachable!("resolved before source mapping")
Expand Down Expand Up @@ -311,7 +321,7 @@ fn ladder(env: &TargetEnv<'_>, session: &Session) -> Result<(Utf8PathBuf, RootSo
}

// Step 6: the picker.
(None, None) => picker(env, session),
(None, None) => picker(env),
}
}

Expand Down Expand Up @@ -531,22 +541,15 @@ fn apply_conflict_choice(

/// The ladder's last step: pick from every known workspace.
///
/// The choice is recorded as the session's new active workspace: an
/// unrecordable choice would be re-made on every invocation, which is why the
/// session layer requires a session identity at all.
fn picker(env: &TargetEnv<'_>, session: &Session) -> Result<(Utf8PathBuf, RootSource)> {
/// The choice resolves this run only.
/// Taking no session identity is the point: a fallback the user was never asked
/// to keep cannot quietly become the answer to every later run from the same
/// terminal.
fn picker(env: &TargetEnv<'_>) -> Result<(Utf8PathBuf, RootSource)> {
let Some(selected) = workspace_target::pick_known_workspace(env, "Select a workspace")? else {
return Err(no_workspace_error(env));
};

if let Some(id) = &selected.id
&& let Err(error) = env
.store
.record_selection(session, id, &selected.root, Utc::now())
{
warn!(%error, "Failed to record the workspace selection.");
}

Ok((selected.root, RootSource::Picker))
}

Expand Down
6 changes: 3 additions & 3 deletions crates/jp_cli/src/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ impl Commands {
/// Declare what this command needs from the workspace bootstrap (RFD 087).
///
/// The workspace-level analog of [`Self::conversation_load_request`]: the
/// bootstrap step only runs workspace resolution when the command asks for
/// it.
/// declaration drives the startup dispatch, so a command reaches exactly
/// the pre-workspace work it asked for.
pub(crate) fn workspace_requirement(&self) -> WorkspaceRequirement {
match self {
Commands::Init(_) => WorkspaceRequirement::None,
Commands::Workspace(args) => args.workspace_requirement(),
Commands::Workspace(_) => WorkspaceRequirement::Subject,
Commands::Query(_)
| Commands::Config(_)
| Commands::Conversation(_)
Expand Down
6 changes: 4 additions & 2 deletions crates/jp_cli/src/cmd/conversation/use_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ use crate::{

/// Set the active conversation.
///
/// Without flags, `jp c use [ID]` activates the given conversation (or opens a
/// picker when no target is provided).
/// `jp c use <ID>` activates the given conversation.
/// Bare `jp c use` returns to the session's previously active conversation, the
/// way `cd -` returns to the previous directory, and opens a picker only when
/// the session has no previous conversation to return to.
/// `--grep` and `--created-since`/`--created-before` restrict the picker's
/// candidate set; when the combined filter leaves a single conversation, it is
/// activated directly without prompting.
Expand Down
18 changes: 1 addition & 17 deletions crates/jp_cli/src/cmd/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ use jp_printer::Printer;
use jp_workspace::session::Session;
use target::{TargetEnv, WorkspaceTarget};

use crate::{
bootstrap::WorkspaceRequirement,
cmd::{self, Output},
};
use crate::cmd::{self, Output};

/// Manage workspaces.
#[derive(Debug, clap::Args)]
Expand Down Expand Up @@ -72,19 +69,6 @@ impl Workspace {
Commands::Show(args) => args.run(printer, &env, persist),
}
}

/// What each subcommand needs from the workspace bootstrap (RFD 087).
///
/// `ls` reads the user-global registries only; `use` resolves and validates
/// a target root to record a selection; `show` additionally loads
/// conversation indexes for its count.
pub(crate) fn workspace_requirement(&self) -> WorkspaceRequirement {
match &self.command {
Commands::Ls(_) => WorkspaceRequirement::None,
Commands::Use(_) => WorkspaceRequirement::Resolve,
Commands::Show(_) => WorkspaceRequirement::Load,
}
}
}

impl Commands {
Expand Down
2 changes: 1 addition & 1 deletion crates/jp_cli/src/cmd/workspace/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ impl Show {
Ok(Some(subject_for(env, id, "session history")))
}

WorkspaceTarget::Latest => Ok(roots::known_workspaces(
WorkspaceTarget::Recent => Ok(roots::known_workspaces(
&env.workspaces_dir,
DEFAULT_STORAGE_DIR,
)
Expand Down
14 changes: 7 additions & 7 deletions crates/jp_cli/src/cmd/workspace/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
//! | `?` | pick from all known workspaces |
//! | `?s`, `?session` | pick from this session's workspace history |
//! | `s`, `session` | the previously active workspace (like `cd -`) |
//! | `l`, `latest` | the most recently used known workspace |
//! | `r`, `recent` | the most recently used known workspace |
//! | `cwd`, `.` | the cwd-derived workspace (as a `use` target: clear) |
//! | `-` | read a workspace ID from stdin |
//! | `help` | print keyword help |
Expand Down Expand Up @@ -64,9 +64,9 @@ pub(crate) enum WorkspaceTarget {
/// `s` / `session` — the session's previously active workspace.
Session,

/// `l` / `latest` — the live root with the newest `last_used` across the
/// `r` / `recent` — the live root with the newest `last_used` across the
/// roots registry (global recency, distinct from `s`).
Latest,
Recent,

/// `cwd` / `.` — the cwd-derived workspace.
///
Expand Down Expand Up @@ -98,7 +98,7 @@ impl FromStr for WorkspaceTarget {
"?" => Self::Picker,
"?s" | "?session" => Self::SessionPicker,
"s" | "session" => Self::Session,
"l" | "latest" => Self::Latest,
"r" | "recent" => Self::Recent,
"cwd" | "." => Self::Cwd,
"-" => Self::Stdin,
"help" => Self::Help,
Expand All @@ -119,7 +119,7 @@ pub(crate) fn help() -> String {
? pick from all known workspaces
?s, ?session pick from this session's workspace history
s, session the previously active workspace (like `cd -`)
l, latest the most recently used known workspace
r, recent the most recently used known workspace
cwd, . the cwd-derived workspace (as a `use` target: clears
the session selection)
- read a workspace ID from stdin
Expand Down Expand Up @@ -314,7 +314,7 @@ pub(crate) fn resolve(target: &WorkspaceTarget, env: &TargetEnv<'_>) -> Result<R
pick("Select a workspace", rows).map(ResolvedTarget::Root)
}

WorkspaceTarget::Latest => latest_root(env)
WorkspaceTarget::Recent => recent_root(env)
.ok_or(no_known_workspaces().into())
.map(ResolvedTarget::Root),

Expand Down Expand Up @@ -407,7 +407,7 @@ pub(crate) fn pick_known_workspace(
}

/// The live root with the newest `last_used` across every known workspace.
fn latest_root(env: &TargetEnv<'_>) -> Option<SelectedRoot> {
fn recent_root(env: &TargetEnv<'_>) -> Option<SelectedRoot> {
// `known_workspaces` orders by most recently used checkout, rootless
// workspaces last, so the first workspace with a root holds the answer.
roots::known_workspaces(&env.workspaces_dir, DEFAULT_STORAGE_DIR)
Expand Down
16 changes: 8 additions & 8 deletions crates/jp_cli/src/cmd/workspace/target_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,12 @@ fn keywords_parse() {
WorkspaceTarget::Session
));
assert!(matches!(
WorkspaceTarget::from_str("l").unwrap(),
WorkspaceTarget::Latest
WorkspaceTarget::from_str("r").unwrap(),
WorkspaceTarget::Recent
));
assert!(matches!(
WorkspaceTarget::from_str("latest").unwrap(),
WorkspaceTarget::Latest
WorkspaceTarget::from_str("recent").unwrap(),
WorkspaceTarget::Recent
));
assert!(matches!(
WorkspaceTarget::from_str("cwd").unwrap(),
Expand Down Expand Up @@ -301,27 +301,27 @@ fn session_target_errors_without_a_previous_entry() {
}

#[test]
fn latest_resolves_the_newest_live_checkout() {
fn recent_resolves_the_newest_live_checkout() {
let tmp = tempdir().unwrap();
let a = make_workspace(tmp.path(), "a", "aaa11");
let b = make_workspace(tmp.path(), "b", "bbb22");
let env = env_at(tmp.path().to_owned(), tmp.path(), None, false);
register_at(&env, "a", "aaa11", &a, 1_000);
register_at(&env, "b", "bbb22", &b, 2_000);

let ResolvedTarget::Root(selected) = resolve(&WorkspaceTarget::Latest, &env).unwrap() else {
let ResolvedTarget::Root(selected) = resolve(&WorkspaceTarget::Recent, &env).unwrap() else {
panic!("expected a resolved root");
};
assert_eq!(selected.root, b);
assert_eq!(selected.id, Some(Id::from_str("bbb22").unwrap()));
}

#[test]
fn latest_errors_with_an_empty_registry() {
fn recent_errors_with_an_empty_registry() {
let tmp = tempdir().unwrap();
let env = env_at(tmp.path().to_owned(), tmp.path(), None, false);

let error = resolve(&WorkspaceTarget::Latest, &env).unwrap_err();
let error = resolve(&WorkspaceTarget::Recent, &env).unwrap_err();
assert!(
message_of(&error).contains("No known workspaces"),
"unexpected error: {error:?}"
Expand Down
43 changes: 34 additions & 9 deletions crates/jp_cli/src/cmd/workspace/use_.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use chrono::Utc;
use crossterm::style::Stylize as _;
use jp_printer::Printer;
use tracing::warn;
use tracing::{debug, warn};

use crate::cmd::{
Output,
Expand All @@ -12,8 +12,11 @@ use crate::cmd::{
///
/// After `jp w use`, workspace-consuming commands run against the selection
/// from anywhere, the way an active conversation follows the session (RFD 020).
/// `jp w use ?` opens a picker; `jp w use cwd` drops the selection and returns
/// to cwd resolution.
/// Bare `jp w use` returns to the session's previously active workspace, the
/// way `cd -` returns to the previous directory, and opens the picker only when
/// the session has no previous workspace to return to.
/// `jp w use ?` always picks; `jp w use cwd` drops the selection and returns to
/// cwd resolution.
///
/// Interactive-only in every form — including `cwd` — because it mutates
/// session state; scripts target a workspace per invocation with `jp
Expand All @@ -24,7 +27,8 @@ pub(crate) struct Use {
/// See `jp w use help` for the grammar.
///
/// Also settable with the global `--workspace` flag, but not both at once.
/// Defaults to the picker (`?`).
/// Defaults to the previously active workspace (`s`), or the picker (`?`)
/// when the session has none.
pub(super) target: Option<WorkspaceTarget>,

/// Keep using this workspace even from inside another one.
Expand All @@ -42,9 +46,7 @@ pub(crate) struct Use {

impl Use {
pub(crate) fn run(self, printer: &Printer, env: &TargetEnv<'_>) -> Output {
let target = self.target.unwrap_or(WorkspaceTarget::Picker);

if matches!(target, WorkspaceTarget::Help) {
if matches!(self.target, Some(WorkspaceTarget::Help)) {
printer.println(target::help());
return Ok(());
}
Expand Down Expand Up @@ -72,7 +74,9 @@ impl Use {

// `cwd` drops the record the sticky flag would live on, so the two
// together ask for a selection that is both absent and permanent.
if self.always && matches!(target, WorkspaceTarget::Cwd) {
// Only an explicit target can be `cwd`; the bare default below never
// resolves to it.
if self.always && matches!(self.target, Some(WorkspaceTarget::Cwd)) {
return Err(format!(
"`{}` clears the session-active workspace, so there is nothing for `{}` to keep \
active.",
Expand All @@ -87,7 +91,7 @@ impl Use {
let previous = mapping.and_then(|mapping| mapping.history.into_iter().next());
let suffix = sticky_suffix(self.always, was_sticky);

match target::resolve(&target, env)? {
match resolve_target(self.target, env)? {
ResolvedTarget::Help => unreachable!("handled before resolution"),

// Clearing is just selecting the cwd-derived workspace: the
Expand Down Expand Up @@ -173,6 +177,27 @@ impl Use {
}
}

/// Resolve the workspace this invocation selects.
///
/// A bare invocation returns to the workspace the session came from, like `cd
/// -`, and falls back to the picker when there is no live one to return to.
fn resolve_target(
target: Option<WorkspaceTarget>,
env: &TargetEnv<'_>,
) -> crate::error::Result<ResolvedTarget> {
if let Some(target) = target {
return target::resolve(&target, env);
}

match target::resolve(&WorkspaceTarget::Session, env) {
Ok(resolved) => Ok(resolved),
Err(error) => {
debug!(%error, "No previous workspace to return to; picking instead.");
target::resolve(&WorkspaceTarget::Picker, env)
}
}
}

/// The trailing clause naming what the invocation did to the session's sticky
/// flag, empty when it leaves the flag off.
fn sticky_suffix(always: bool, was_sticky: bool) -> &'static str {
Expand Down
Loading
Loading