Skip to content

Commit 056fdf6

Browse files
Byroncodex
andcommitted
Make ChangeIds mandatory for commits in but_workspace::ui::RefInfo
Co-authored-by: GPT 5.4 <codex@openai.com>
1 parent 91204b4 commit 056fdf6

7 files changed

Lines changed: 64 additions & 20 deletions

File tree

crates/but-core/src/commit.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ impl Headers {
5959
if self.change_id.is_none() {
6060
self.change_id = commit_id
6161
.into()
62-
.map_or_else(ChangeId::generate_sha1, Self::synthetic_change_id_from_commit_id)
62+
.map_or_else(
63+
ChangeId::generate_sha1,
64+
Self::synthetic_change_id_from_commit_id,
65+
)
6366
.into();
6467
}
6568
self
@@ -528,6 +531,15 @@ impl CommitOwned {
528531
inner,
529532
}
530533
}
534+
535+
/// Return the stored change-id if present, or derive a deterministic fallback from the commit id.
536+
pub fn change_id(&self) -> ChangeId {
537+
Headers::try_from_commit(&self.inner)
538+
.unwrap_or_default()
539+
.ensure_change_id(self.id)
540+
.change_id
541+
.expect("change-id is ensured")
542+
}
531543
}
532544

533545
/// Mutations
@@ -604,6 +616,15 @@ impl<'repo> Commit<'repo> {
604616
pub fn headers(&self) -> Option<Headers> {
605617
Headers::try_from_commit(&self.inner)
606618
}
619+
620+
/// Return the stored change-id if present, or derive a deterministic fallback from the commit id.
621+
pub fn change_id(&self) -> ChangeId {
622+
self.headers()
623+
.unwrap_or_default()
624+
.ensure_change_id(self.id.detach())
625+
.change_id
626+
.expect("change-id is ensured")
627+
}
607628
}
608629

609630
/// Conflict specific details

crates/but-workspace/src/branch_details.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,7 @@ fn local_commits_gix(
234234
state: CommitState::LocalAndRemote(info.id),
235235
created_at: i128::from(commit.committer.time.seconds) * 1000,
236236
author,
237-
change_id: commit
238-
.headers()
239-
.and_then(|headers| headers.change_id)
240-
.map(|id| id.to_string()),
237+
change_id: commit.change_id().to_string(),
241238
gerrit_review_url: None,
242239
});
243240
}

crates/but-workspace/src/commit_engine/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,8 @@ fn create_possibly_signed_commit(
222222
parents: impl IntoIterator<Item = impl Into<gix::ObjectId>>,
223223
commit_headers: Option<but_core::commit::Headers>,
224224
) -> anyhow::Result<gix::ObjectId> {
225-
let commit_headers = commit_headers.unwrap_or_else(|| {
226-
Headers::from_config(&repo.config_snapshot())
227-
});
225+
let commit_headers =
226+
commit_headers.unwrap_or_else(|| Headers::from_config(&repo.config_snapshot()));
228227
let commit = gix::objs::Commit {
229228
message: message.into(),
230229
tree,

crates/but-workspace/src/legacy/stacks.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,11 @@ pub fn local_and_remote_commits(
627627
state,
628628
created_at,
629629
author: gix_commit.author()?.into(),
630-
change_id: change_id.map(|id| id.to_string()),
630+
change_id: change_id
631+
.unwrap_or_else(|| {
632+
but_core::commit::Headers::synthetic_change_id_from_commit_id(*commit_id)
633+
})
634+
.to_string(),
631635
gerrit_review_url: None,
632636
};
633637
local_and_remote.push(api_commit);

crates/but-workspace/src/ui/mod.rs

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,10 @@ pub struct Commit {
108108
pub created_at: i128,
109109
/// The author of the commit.
110110
pub author: Author,
111-
/// The GitButler change-id associated with this commit, if available.
112-
pub change_id: Option<String>,
111+
/// The GitButler change-id associated with this commit.
112+
/// It always exists as we either read it from the [headers][but_core::commit::Headers], or we
113+
/// synthesize one from [the commit id][but_core::commit::Headers::synthetic_change_id_from_commit_id()].
114+
pub change_id: String,
113115
/// Optional URL to the Gerrit review for this commit, if applicable.
114116
/// Only populated if Gerrit mode is enabled and the commit has an associated review.
115117
pub gerrit_review_url: Option<String>,
@@ -123,17 +125,22 @@ impl TryFrom<gix::Commit<'_>> for Commit {
123125
let commit_id = commit.id;
124126
let commit = commit.decode()?;
125127
let headers = but_core::commit::Headers::try_from_commit_headers(|| commit.extra_headers());
128+
let has_conflicts = headers.as_ref().is_some_and(|hdr| hdr.is_conflicted());
129+
let change_id = headers
130+
.unwrap_or_default()
131+
.ensure_change_id(commit_id)
132+
.change_id
133+
.expect("change-id is ensured")
134+
.to_string();
126135
Ok(Commit {
127136
id: commit_id,
128137
parent_ids: commit.parents().collect(),
129138
message: commit.message.to_owned(),
130-
has_conflicts: headers.as_ref().is_some_and(|hdr| hdr.is_conflicted()),
139+
has_conflicts,
131140
state: CommitState::LocalAndRemote(commit_id),
132141
created_at: i128::from(commit.time()?.seconds) * 1000,
133142
author: commit.author()?.into(),
134-
change_id: headers
135-
.and_then(|headers| headers.change_id)
136-
.map(|id| id.to_string()),
143+
change_id,
137144
gerrit_review_url: None,
138145
})
139146
}
@@ -144,8 +151,11 @@ impl From<but_core::CommitOwned> for Commit {
144151
let headers = commit::Headers::try_from_commit(&inner);
145152
let has_conflicts = headers.as_ref().is_some_and(|hdr| hdr.is_conflicted());
146153
let change_id = headers
147-
.and_then(|hdr| hdr.change_id)
148-
.map(|id| id.to_string());
154+
.unwrap_or_default()
155+
.ensure_change_id(id)
156+
.change_id
157+
.expect("change-id is ensured")
158+
.to_string();
149159
let gix::objs::Commit {
150160
tree: _,
151161
parents,
@@ -439,7 +449,12 @@ impl From<&LocalCommit> for ui::Commit {
439449
author: author
440450
.to_ref(&mut gix::date::parse::TimeBuf::default())
441451
.into(),
442-
change_id: change_id.as_ref().map(ToString::to_string),
452+
change_id: change_id
453+
.as_ref()
454+
.map(ToString::to_string)
455+
.unwrap_or_else(|| {
456+
but_core::commit::Headers::synthetic_change_id_from_commit_id(*id).to_string()
457+
}),
443458
gerrit_review_url: None,
444459
}
445460
}

crates/but-workspace/tests/workspace/ui.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ mod changes_in_branch {
259259
"email": "author@example.com",
260260
"gravatarUrl": "https://www.gravatar.com/avatar/5c1e6d6e64e12aca17657581a48005d1?s=100&r=g&d=retro"
261261
},
262-
"changeId": "rwzxluzvxmyzkuqmpzovmuwqsxptxrpn"
262+
"changeId": null
263263
}
264264
],
265265
"commitsOutside": null,
@@ -444,7 +444,7 @@ mod changes_in_branch {
444444
"email": "author@example.com",
445445
"gravatarUrl": "https://www.gravatar.com/avatar/5c1e6d6e64e12aca17657581a48005d1?s=100&r=g&d=retro"
446446
},
447-
"changeId": "rwzxluzvxmyzkuqmpzovmuwqsxptxrpn"
447+
"changeId": null
448448
}
449449
],
450450
"commitsOutside": null,

packages/but-sdk/src/generated/index.d.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,12 @@ export type Commit = {
581581
createdAt: number;
582582
/** The author of the commit. */
583583
author: Author;
584+
/**
585+
* The GitButler change-id associated with this commit.
586+
* It always exists as we either read it from the [headers][but_core::commit::Headers], or we
587+
* synthesize one from [the commit id][but_core::commit::Headers::synthetic_change_id_from_commit_id()].
588+
*/
589+
changeId: string;
584590
/**
585591
* Optional URL to the Gerrit review for this commit, if applicable.
586592
* Only populated if Gerrit mode is enabled and the commit has an associated review.
@@ -1489,6 +1495,8 @@ export type UpstreamCommit = {
14891495
createdAt: number;
14901496
/** The author of the commit. */
14911497
author: Author;
1498+
/** The GitButler change-id associated with this commit, if available. */
1499+
changeId?: string | null;
14921500
};
14931501

14941502
/** Git files activity. Supplies the head sha */

0 commit comments

Comments
 (0)