Skip to content

Fix CommitTitle to show context-appropriate messages for empty commits #10406

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+9 −3
Open

Fix CommitTitle to show context-appropriate messages for empty commits #10406

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
945ef1c
Add editable prop to CommitTitle component for context-aware messages
Copilot Sep 22, 2025
e859f23
refactor
Byron Sep 22, 2025
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
2 changes: 2 additions & 0 deletions apps/desktop/src/components/BranchCommitList.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@
{active}
onclick={() => handleCommitClick(commit.id, true)}
disableCommitActions={false}
editable={!!stackId}
/>
{/if}
{/each}
Expand Down Expand Up @@ -428,6 +429,7 @@
{active}
onclick={() => handleCommitClick(commit.id, false)}
disableCommitActions={false}
editable={!!stackId}
>
{#snippet menu({ rightClickTrigger })}
{@const data = {
Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/src/components/BranchesViewBranch.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
});
}}
lastCommit={idx === branch.upstreamCommits.length - 1 && branch.commits.length === 0}
editable={false}
/>
{/each}
{#each branch.commits || [] as commit, idx}
Expand All @@ -108,6 +109,7 @@
}}
lastCommit={idx === branch.commits.length - 1}
active
editable={false}
/>
{/each}
</div>
Expand Down
4 changes: 3 additions & 1 deletion apps/desktop/src/components/CommitRow.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
active?: boolean;
hasConflicts?: boolean;
disabled?: boolean;
editable?: boolean;
menu?: Snippet<[{ rightClickTrigger: HTMLElement }]>;
onclick?: () => void;
};
Expand Down Expand Up @@ -74,6 +75,7 @@
disabled,
hasConflicts,
active,
editable = true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, let's remove this default value here. The type is already editable?: boolean; so instances don't need the prop specified as false. This means we could remove the explicit editable={false} rows in this pr.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! I think I have addressed the this as well as I could, given that… somehow editable = false was needed so editable wouldn't be truthy by default. (I don't understand what's going on here, I hope I am missing something)

In any case, I hope you will merge this PR after taking it to the finishing line 🙏.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure thing, I can I can take of the rest, just ignore my new comment! It shouldn't be truthy by default, but I can make sure. :)

onclick,
menu,
...args
Expand Down Expand Up @@ -144,7 +146,7 @@
{/if}

<div class="commit-name truncate">
<CommitTitle {commitMessage} truncate className="text-13 text-semibold" />
<CommitTitle {commitMessage} truncate className="text-13 text-semibold" {editable} />
</div>

{#if !args.disableCommitActions}
Expand Down
5 changes: 3 additions & 2 deletions apps/desktop/src/components/CommitTitle.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@
truncate?: boolean;
commitMessage: string;
className?: string;
editable?: boolean;
};

const { commitMessage, truncate, className }: Props = $props();
const { commitMessage, truncate, className, editable = true }: Props = $props();

const title = $derived(splitMessage(commitMessage).title);

function getTitle() {
if (title) {
return title;
}
return 'Empty commit. Drag changes here';
return editable ? 'Empty commit. Drag changes here' : 'Empty commit';
}
</script>

Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/components/CommitView.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@
truncate
commitMessage={commit.message}
className="text-14 text-semibold text-body"
editable={!isReadOnly}
/>
{/snippet}

Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/components/TargetCommitList.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
commitMessage={commit.message}
createdAt={commit.createdAt}
author={commit.author}
editable={false}
onclick={() => {
branchesState.set({
commitId: commit.id,
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/components/UnappliedCommitView.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
truncate
commitMessage={commit.message}
className="text-14 text-semibold text-body"
editable={false}
/>
{/snippet}

Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/components/codegen/CodegenPage.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,7 @@
lastCommit={i === commits.length - 1}
{lastBranch}
tooltip={commitStatusLabel(commit.state.type)}
editable={false}
/>
{/each}
{/snippet}
Expand Down
Loading