Skip to content

Commit ad7a8b7

Browse files
ByronByron
authored
Merge pull request #11508 from gitbutlerapp/copilot/forbid-missing-documentation
Forbid missing documentation in crates/but/src/id/mod.rs
2 parents 59c5a4b + a47d03d commit ad7a8b7

File tree

12 files changed

+350
-198
lines changed

12 files changed

+350
-198
lines changed

‎crates/but/src/command/legacy/branch/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub async fn handle(
9191
// Use the new create_reference API when anchor is provided
9292

9393
// Resolve the anchor string to a CliId
94-
let anchor_ids = id_map.parse_str(&anchor_str)?;
94+
let anchor_ids = id_map.resolve_entity_to_ids(&anchor_str)?;
9595
if anchor_ids.is_empty() {
9696
return Err(anyhow::anyhow!("Could not find anchor: {}", anchor_str));
9797
}
@@ -106,7 +106,7 @@ pub async fn handle(
106106
// Create the anchor for create_reference
107107
// as dependent branch
108108
match anchor_id {
109-
CliId::Commit { oid } => {
109+
CliId::Commit(oid) => {
110110
Some(but_api::legacy::stack::create_reference::Anchor::AtCommit {
111111
commit_id: (*oid).into(),
112112
position: but_workspace::branch::create_reference::Position::Above,

‎crates/but/src/command/legacy/commit.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub(crate) fn insert_blank_commit(
2525
id_map.add_file_info_from_context(&mut ctx)?;
2626

2727
// Resolve the target ID
28-
let cli_ids = id_map.parse_str(target)?;
28+
let cli_ids = id_map.resolve_entity_to_ids(target)?;
2929

3030
if cli_ids.is_empty() {
3131
bail!("Target '{}' not found", target);
@@ -43,7 +43,7 @@ pub(crate) fn insert_blank_commit(
4343

4444
// Determine target commit ID and offset based on CLI ID type
4545
let (target_commit_id, offset, success_message) = match cli_id {
46-
CliId::Commit { oid } => {
46+
CliId::Commit(oid) => {
4747
// For commits, insert before (offset 0) and use the commit ID directly
4848
(
4949
*oid,
@@ -240,7 +240,7 @@ pub(crate) fn commit(
240240
.find(|branch| branch.name == hint)
241241
.or_else(|| {
242242
// If no exact match, try to parse as CLI ID and match
243-
if let Ok(cli_ids) = id_map.parse_str(hint) {
243+
if let Ok(cli_ids) = id_map.resolve_entity_to_ids(hint) {
244244
for cli_id in cli_ids {
245245
if let CliId::Branch { name, .. } = cli_id
246246
&& let Some(branch) =
@@ -418,7 +418,7 @@ fn find_stack_by_hint(
418418
}
419419

420420
// Try CLI ID parsing
421-
let cli_ids = id_map.parse_str(hint).ok()?;
421+
let cli_ids = id_map.resolve_entity_to_ids(hint).ok()?;
422422
for cli_id in cli_ids {
423423
if let CliId::Branch { name, .. } = cli_id {
424424
for (stack_id, stack_details) in stacks {

‎crates/but/src/command/legacy/describe.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub(crate) fn describe_target(
1818
id_map.add_file_info_from_context(&mut ctx)?;
1919

2020
// Resolve the commit ID
21-
let cli_ids = id_map.parse_str(target)?;
21+
let cli_ids = id_map.resolve_entity_to_ids(target)?;
2222

2323
if cli_ids.is_empty() {
2424
bail!("ID '{}' not found", target);
@@ -38,7 +38,7 @@ pub(crate) fn describe_target(
3838
CliId::Branch { name, .. } => {
3939
edit_branch_name(&ctx, project, name, out, message)?;
4040
}
41-
CliId::Commit { oid } => {
41+
CliId::Commit(oid) => {
4242
edit_commit_message_by_id(&ctx, project, *oid, out, message)?;
4343
}
4444
_ => {

‎crates/but/src/command/legacy/forge/review.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ fn get_branch_names(project: &Project, branch_id: &str) -> anyhow::Result<Vec<St
9797
let mut id_map = IdMap::new_from_context(&ctx)?;
9898
id_map.add_file_info_from_context(&mut ctx)?;
9999
let branch_ids = id_map
100-
.parse_str(branch_id)?
100+
.resolve_entity_to_ids(branch_id)?
101101
.iter()
102102
.filter_map(|clid| match clid {
103103
CliId::Branch { name, .. } => Some(name.clone()),

‎crates/but/src/command/legacy/mark.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub(crate) fn handle(
1818
let ctx = &mut Context::new_from_legacy_project(project.clone())?;
1919
let mut id_map = IdMap::new_from_context(ctx)?;
2020
id_map.add_file_info_from_context(ctx)?;
21-
let target_result = id_map.parse_str(target_str)?;
21+
let target_result = id_map.resolve_entity_to_ids(target_str)?;
2222
if target_result.len() != 1 {
2323
return Err(anyhow::anyhow!(
2424
"Target {} is ambiguous: {:?}",
@@ -32,7 +32,7 @@ pub(crate) fn handle(
3232
}
3333
match target_result[0].clone() {
3434
CliId::Branch { name, .. } => mark_branch(ctx, name, delete, out),
35-
CliId::Commit { oid } => mark_commit(ctx, oid, delete, out),
35+
CliId::Commit(oid) => mark_commit(ctx, oid, delete, out),
3636
_ => bail!("Nope"),
3737
}
3838
}

‎crates/but/src/command/legacy/push.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ fn resolve_branch_name(
119119
branch_id: &str,
120120
) -> anyhow::Result<String> {
121121
// Try to resolve as CliId first
122-
let cli_ids = id_map.parse_str(branch_id)?;
122+
let cli_ids = id_map.resolve_entity_to_ids(branch_id)?;
123123

124124
if cli_ids.is_empty() {
125125
// If no CliId matches, treat as literal branch name but validate it exists

‎crates/but/src/command/legacy/rub/mod.rs‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ pub(crate) fn handle(
4040
CliId::UncommittedFile {
4141
path, assignment, ..
4242
},
43-
CliId::Commit { oid },
43+
CliId::Commit(oid),
4444
) => {
4545
create_snapshot(ctx, OperationKind::AmendCommit);
4646
amend::file_to_commit(ctx, path.as_ref(), *assignment, oid, out)?;
@@ -55,7 +55,7 @@ pub(crate) fn handle(
5555
(CliId::Unassigned { .. }, CliId::Unassigned { .. }) => {
5656
bail!(makes_no_sense_error(&source, &target))
5757
}
58-
(CliId::Unassigned { .. }, CliId::Commit { oid }) => {
58+
(CliId::Unassigned { .. }, CliId::Commit(oid)) => {
5959
create_snapshot(ctx, OperationKind::AmendCommit);
6060
amend::assignments_to_commit(ctx, None, oid, out)?;
6161
}
@@ -66,15 +66,15 @@ pub(crate) fn handle(
6666
(CliId::Commit { .. }, CliId::UncommittedFile { .. }) => {
6767
bail!(makes_no_sense_error(&source, &target))
6868
}
69-
(CliId::Commit { oid }, CliId::Unassigned { .. }) => {
69+
(CliId::Commit(oid), CliId::Unassigned { .. }) => {
7070
create_snapshot(ctx, OperationKind::UndoCommit);
7171
undo::commit(ctx, oid, out)?;
7272
}
73-
(CliId::Commit { oid: source }, CliId::Commit { oid: destination }) => {
73+
(CliId::Commit(source), CliId::Commit(destination)) => {
7474
create_snapshot(ctx, OperationKind::SquashCommit);
7575
squash::commits(ctx, source, destination, out)?;
7676
}
77-
(CliId::Commit { oid }, CliId::Branch { name, .. }) => {
77+
(CliId::Commit(oid), CliId::Branch { name, .. }) => {
7878
create_snapshot(ctx, OperationKind::MoveCommit);
7979
move_commit::to_branch(ctx, oid, name, out)?;
8080
}
@@ -85,7 +85,7 @@ pub(crate) fn handle(
8585
create_snapshot(ctx, OperationKind::MoveHunk);
8686
assign::assign_all(ctx, Some(from), None, out)?;
8787
}
88-
(CliId::Branch { name, .. }, CliId::Commit { oid }) => {
88+
(CliId::Branch { name, .. }, CliId::Commit(oid)) => {
8989
create_snapshot(ctx, OperationKind::AmendCommit);
9090
amend::assignments_to_commit(ctx, Some(name), oid, out)?;
9191
}
@@ -104,7 +104,9 @@ pub(crate) fn handle(
104104
}
105105
(
106106
CliId::CommittedFile {
107-
path, commit_oid, ..
107+
path,
108+
commit_id: commit_oid,
109+
..
108110
},
109111
CliId::Branch { name, .. },
110112
) => {
@@ -113,9 +115,11 @@ pub(crate) fn handle(
113115
}
114116
(
115117
CliId::CommittedFile {
116-
path, commit_oid, ..
118+
path,
119+
commit_id: commit_oid,
120+
..
117121
},
118-
CliId::Commit { oid },
122+
CliId::Commit(oid),
119123
) => {
120124
create_snapshot(ctx, OperationKind::FileChanges);
121125
commits::commited_file_to_another_commit(
@@ -128,7 +132,9 @@ pub(crate) fn handle(
128132
}
129133
(
130134
CliId::CommittedFile {
131-
path, commit_oid, ..
135+
path,
136+
commit_id: commit_oid,
137+
..
132138
},
133139
CliId::Unassigned { .. },
134140
) => {
@@ -166,7 +172,7 @@ fn ids(
166172
target: &str,
167173
) -> anyhow::Result<(Vec<CliId>, CliId)> {
168174
let sources = parse_sources(ctx, id_map, source)?;
169-
let target_result = id_map.parse_str(target)?;
175+
let target_result = id_map.resolve_entity_to_ids(target)?;
170176
if target_result.len() != 1 {
171177
if target_result.is_empty() {
172178
return Err(anyhow::anyhow!(
@@ -177,7 +183,7 @@ fn ids(
177183
let matches: Vec<String> = target_result
178184
.iter()
179185
.map(|id| match id {
180-
CliId::Commit { oid } => {
186+
CliId::Commit(oid) => {
181187
format!("{} (commit {})", id, &oid.to_string()[..7])
182188
}
183189
CliId::Branch { name, .. } => format!("{id} (branch '{name}')"),
@@ -209,7 +215,7 @@ pub(crate) fn parse_sources(
209215
}
210216
// Single source
211217
else {
212-
let source_result = id_map.parse_str(source)?;
218+
let source_result = id_map.resolve_entity_to_ids(source)?;
213219
if source_result.len() != 1 {
214220
if source_result.is_empty() {
215221
return Err(anyhow::anyhow!(
@@ -220,7 +226,7 @@ pub(crate) fn parse_sources(
220226
let matches: Vec<String> = source_result
221227
.iter()
222228
.map(|id| match id {
223-
CliId::Commit { oid } => {
229+
CliId::Commit(oid) => {
224230
format!("{} (commit {})", id, &oid.to_string()[..7])
225231
}
226232
CliId::Branch { name, .. } => format!("{id} (branch '{name}')"),
@@ -251,8 +257,8 @@ fn parse_range(ctx: &mut Context, id_map: &IdMap, source: &str) -> anyhow::Resul
251257
let end_str = parts[1];
252258

253259
// Get the start and end IDs
254-
let start_matches = id_map.parse_str(start_str)?;
255-
let end_matches = id_map.parse_str(end_str)?;
260+
let start_matches = id_map.resolve_entity_to_ids(start_str)?;
261+
let end_matches = id_map.resolve_entity_to_ids(end_str)?;
256262

257263
if start_matches.len() != 1 {
258264
return Err(anyhow::anyhow!(
@@ -329,7 +335,7 @@ fn get_all_files_in_display_order(ctx: &mut Context, id_map: &IdMap) -> anyhow::
329335
if let Some(stack_id) = assignment.stack_id
330336
&& stack.id == Some(stack_id)
331337
{
332-
let file_id = id_map.uncommitted_file(
338+
let file_id = id_map.resolve_uncommitted_file_or_unassigned(
333339
assignment.stack_id,
334340
assignment.path_bytes.as_ref(),
335341
);
@@ -347,7 +353,8 @@ fn get_all_files_in_display_order(ctx: &mut Context, id_map: &IdMap) -> anyhow::
347353
for assignments in by_file.values() {
348354
for assignment in assignments {
349355
if assignment.stack_id.is_none() {
350-
let file_id = id_map.uncommitted_file(None, assignment.path_bytes.as_ref());
356+
let file_id = id_map
357+
.resolve_uncommitted_file_or_unassigned(None, assignment.path_bytes.as_ref());
351358
if !all_files.contains(&file_id) {
352359
all_files.push(file_id);
353360
}
@@ -364,7 +371,7 @@ fn parse_list(id_map: &IdMap, source: &str) -> anyhow::Result<Vec<CliId>> {
364371

365372
for part in parts {
366373
let part = part.trim();
367-
let matches = id_map.parse_str(part)?;
374+
let matches = id_map.resolve_entity_to_ids(part)?;
368375
if matches.len() != 1 {
369376
if matches.is_empty() {
370377
return Err(anyhow::anyhow!(

‎crates/but/src/command/legacy/status/assignment.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ pub struct CLIHunkAssignment {
1616
impl CLIHunkAssignment {
1717
fn from_assignment(id_map: &IdMap, inner: HunkAssignment) -> Self {
1818
let cli_id = id_map
19-
.uncommitted_file(inner.stack_id, inner.path_bytes.as_ref())
19+
.resolve_uncommitted_file_or_unassigned(inner.stack_id, inner.path_bytes.as_ref())
2020
.to_string();
2121
Self { inner, cli_id }
2222
}

‎crates/but/src/command/legacy/status/json.rs‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ impl Branch {
177177
.iter()
178178
.map(|c| {
179179
Commit::from_local_commit(
180-
CliId::commit(c.id).to_string(),
180+
CliId::Commit(c.id).to_string(),
181181
c.clone(),
182182
show_files,
183183
project_id,
@@ -189,7 +189,7 @@ impl Branch {
189189
let upstream_commits = branch
190190
.upstream_commits
191191
.iter()
192-
.map(|c| Commit::from_upstream_commit(CliId::commit(c.id).to_string(), c.clone(), None))
192+
.map(|c| Commit::from_upstream_commit(CliId::Commit(c.id).to_string(), c.clone(), None))
193193
.collect();
194194

195195
Ok(Branch {
@@ -231,7 +231,10 @@ impl Commit {
231231
.changes
232232
.into_iter()
233233
.map(|change| {
234-
let cli_id = id_map.committed_file(commit.id, change.path.as_ref());
234+
let cli_id = id_map.resolve_file_changed_in_commit_or_unassigned(
235+
commit.id,
236+
change.path.as_ref(),
237+
);
235238
FileChange::from_tree_change(cli_id.to_string(), change)
236239
})
237240
.collect(),
@@ -329,7 +332,9 @@ fn convert_branch_to_json(
329332
review_map: &std::collections::HashMap<String, Vec<but_forge::ForgeReview>>,
330333
id_map: &mut crate::IdMap,
331334
) -> anyhow::Result<Branch> {
332-
let cli_id = id_map.branch(branch.name.as_ref()).to_string();
335+
let cli_id = id_map
336+
.resolve_branch_or_insert(branch.name.as_ref())
337+
.to_string();
333338

334339
let review_id = if review {
335340
crate::command::legacy::forge::review::get_review_numbers(
@@ -385,7 +390,7 @@ pub(super) fn build_workspace_status_json(
385390
let stack_cli_id = details
386391
.branch_details
387392
.first()
388-
.map(|b| id_map.branch(b.name.as_ref()).to_string())
393+
.map(|b| id_map.resolve_branch_or_insert(b.name.as_ref()).to_string())
389394
.unwrap_or_else(|| "unknown".to_string());
390395

391396
let json_assigned_changes = convert_file_assignments(assignments, worktree_changes);
@@ -411,7 +416,7 @@ pub(super) fn build_workspace_status_json(
411416
let base_commit_decoded = base_commit.decode()?;
412417
let author: but_workspace::ui::Author = base_commit_decoded.author()?.into();
413418

414-
let cli_id = CliId::commit(common_merge_base.commit_id).to_string();
419+
let cli_id = CliId::Commit(common_merge_base.commit_id).to_string();
415420
let merge_base_commit = Commit::from_upstream_commit(
416421
cli_id,
417422
but_workspace::ui::UpstreamCommit {
@@ -429,7 +434,7 @@ pub(super) fn build_workspace_status_json(
429434
let upstream_commit_decoded = upstream_commit.decode()?;
430435
let upstream_author: but_workspace::ui::Author = upstream_commit_decoded.author()?.into();
431436

432-
let cli_id = CliId::commit(upstream.commit_id).to_string();
437+
let cli_id = CliId::Commit(upstream.commit_id).to_string();
433438
let latest_commit = Commit::from_upstream_commit(
434439
cli_id,
435440
but_workspace::ui::UpstreamCommit {

‎crates/but/src/command/legacy/status/mod.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ pub(crate) async fn worktree(
7171
..Default::default()
7272
},
7373
)?;
74-
let mut id_map = IdMap::new(&head_info.stacks)?;
74+
let mut id_map = IdMap::new_for_branches_and_commits(&head_info.stacks)?;
7575
id_map.add_file_info_from_context(ctx)?;
7676

7777
let review_map = if review {
@@ -389,7 +389,7 @@ pub fn print_group(
389389
let mut first = true;
390390
for branch in &group.branch_details {
391391
let id = id_map
392-
.branch(branch.name.as_ref())
392+
.resolve_branch_or_insert(branch.name.as_ref())
393393
.to_string()
394394
.underline()
395395
.blue();
@@ -490,7 +490,6 @@ pub fn print_group(
490490
}
491491
}
492492
} else {
493-
let mut id_map = IdMap::new_from_context(ctx)?;
494493
id_map.add_file_info_from_context(ctx)?;
495494
let id = id_map.unassigned().to_string().underline().blue();
496495
writeln!(
@@ -666,7 +665,7 @@ fn print_commit(
666665
if show_files {
667666
for change in &commit_details.diff_with_first_parent {
668667
let cid = id_map
669-
.committed_file(commit_id, change.path.as_ref())
668+
.resolve_file_changed_in_commit_or_unassigned(commit_id, change.path.as_ref())
670669
.to_string()
671670
.blue()
672671
.underline();

0 commit comments

Comments
 (0)