Radish alpha
r
Radicle desktop app
Radicle
Git (anonymous pull)
Log in to clone via SSH
Inline review code-comment hunks in activity
Brandon Oxendine committed 1 month ago
commit dd2582a0fa7cf773b9ec2db48f5df93d7dbd1dd1
parent 45bb52180b103893328442a01ae8ffd88e6d8d07
4 files changed +192 -43
modified src/components/Diff.svelte
@@ -26,6 +26,9 @@
    canReply?: boolean;
    // See `ExtendedTextarea`.
    disableAttachments?: boolean | string;
+
    // When `true`, the file:line chip rendered above each inline thread is
+
    // hidden. Useful when the surrounding context already shows the file path.
+
    hideThreadFileHeader?: boolean;
    repoDelegates: Author[];
    rid: string;
    threads: Thread<CodeLocation>[];
@@ -400,10 +403,12 @@
  {#if codeComments && thread && !threadExpandedStates[thread.root.id]}
    <div class="thread">
      <div class="global-flex" style:padding="0.5rem">
-
        {@render commentHeader(
-
          thread.root.location?.path,
-
          rangeAnchorsFromCodeLocation(thread.root.location),
-
        )}
+
        {#if !codeComments.hideThreadFileHeader}
+
          {@render commentHeader(
+
            thread.root.location?.path,
+
            rangeAnchorsFromCodeLocation(thread.root.location),
+
          )}
+
        {/if}
        {#if codeComments.changeCommentStatus && roles.isDelegateOrAuthor( codeComments.config.publicKey, codeComments.repoDelegates.map(delegate => delegate.did), thread.root.author.did, )}
          <div style:margin-left="auto">
            {#if thread.root.resolved}
modified src/components/Discussion.svelte
@@ -110,9 +110,6 @@
    margin-left: 1.25rem;
    background-color: var(--color-border-subtle);
  }
-
  .activity {
-
    padding: 0.25rem 0;
-
  }
</style>

<div style:margin="1.5rem 0 2.5rem 0">
@@ -133,9 +130,7 @@
          {reactOnComment} />
        <div class="connector"></div>
      {:else if renderActivity}
-
        <div class="activity">
-
          {@render renderActivity(entry.data)}
-
        </div>
+
        {@render renderActivity(entry.data)}
        <div class="connector"></div>
      {/if}
    {/each}
added src/components/ReviewCodeThread.svelte
@@ -0,0 +1,117 @@
+
<script lang="ts">
+
  import type { CodeComments } from "@app/components/Diff.svelte";
+
  import type { Author } from "@bindings/cob/Author";
+
  import type { CodeLocation } from "@bindings/cob/thread/CodeLocation";
+
  import type { Thread } from "@bindings/cob/thread/Thread";
+
  import type { Config } from "@bindings/config/Config";
+
  import type { Diff as DiffType } from "@bindings/diff/Diff";
+
  import type { FileDiff } from "@bindings/diff/FileDiff";
+
  import type { Hunk } from "@bindings/diff/Hunk";
+

+
  import { cachedGetDiff } from "@app/lib/invoke";
+

+
  import Diff from "@app/components/Diff.svelte";
+

+
  interface Props {
+
    rid: string;
+
    base: string;
+
    head: string;
+
    thread: Thread<CodeLocation>;
+
    config: Config;
+
    repoDelegates: Author[];
+
  }
+

+
  const { rid, base, head, thread, config, repoDelegates }: Props = $props();
+

+
  const noop = () => Promise.resolve();
+

+
  const codeComments: CodeComments = {
+
    config,
+
    repoDelegates,
+
    rid,
+
    threads: [thread],
+
    canReply: false,
+
    hideThreadFileHeader: true,
+
    createComment: noop,
+
    editComment: noop,
+
  };
+

+
  function findFileDiff(
+
    diff: DiffType,
+
    location: CodeLocation,
+
  ): FileDiff | undefined {
+
    return diff.files.find(file => {
+
      if (file.status === "moved" || file.status === "copied") {
+
        return file.oldPath === location.path || file.newPath === location.path;
+
      }
+
      return file.path === location.path;
+
    });
+
  }
+

+
  function findHunk(file: FileDiff, location: CodeLocation): Hunk | undefined {
+
    if (file.diff.type !== "plain") return undefined;
+
    const range = location.new ?? location.old;
+
    if (!range || range.type !== "lines") return undefined;
+
    const targetSide: "old" | "new" = location.new ? "new" : "old";
+

+
    return file.diff.hunks.find(hunk => {
+
      const hunkRange = hunk[targetSide];
+
      return (
+
        hunkRange.start <= range.range.start && hunkRange.end >= range.range.end
+
      );
+
    });
+
  }
+

+
  function sliceFileDiff(file: FileDiff, hunk: Hunk): FileDiff {
+
    if (file.diff.type !== "plain") return file;
+
    return {
+
      ...file,
+
      diff: { ...file.diff, hunks: [hunk] },
+
    };
+
  }
+
</script>
+

+
<style>
+
  .wrapper {
+
    border: 1px solid var(--color-border-subtle);
+
    border-radius: var(--border-radius-md);
+
    overflow: hidden;
+
  }
+
  .wrapper :global(.container) {
+
    padding-top: 0;
+
    padding-bottom: 0;
+
  }
+
  .file-header {
+
    font: var(--txt-body-m-regular);
+
    color: var(--color-text-secondary);
+
    padding: 0.5rem 0.75rem;
+
    border-bottom: 1px solid var(--color-border-subtle);
+
    background-color: var(--color-surface-canvas);
+
  }
+
  .fallback {
+
    font: var(--txt-body-m-regular);
+
    color: var(--color-text-secondary);
+
  }
+
</style>
+

+
{#await cachedGetDiff(rid, { base, head, unified: 3, highlight: true })}
+
  <div class="fallback">Loading code…</div>
+
{:then diff}
+
  {@const location = thread.root.location}
+
  {#if location}
+
    {@const file = findFileDiff(diff, location)}
+
    {#if file}
+
      {@const hunk = findHunk(file, location)}
+
      {#if hunk}
+
        <div class="wrapper">
+
          <div class="file-header">{location.path}</div>
+
          <Diff file={sliceFileDiff(file, hunk)} {head} {codeComments} />
+
        </div>
+
      {:else}
+
        <div class="fallback">Code unavailable for {location.path}</div>
+
      {/if}
+
    {:else}
+
      <div class="fallback">Code unavailable for {location.path}</div>
+
    {/if}
+
  {/if}
+
{/await}
modified src/components/Revision.svelte
@@ -1,4 +1,5 @@
<script lang="ts">
+
  import type { CodeLocation } from "@bindings/cob/thread/CodeLocation";
  import type { Author } from "@bindings/cob/Author";
  import type { Operation } from "@bindings/cob/Operation";
  import type { Action } from "@bindings/cob/patch/Action";
@@ -21,6 +22,11 @@
  import PatchActivityItem, {
    type FlattenedPatchOperation,
  } from "@app/components/PatchActivityItem.svelte";
+
  import ReviewCodeThread from "@app/components/ReviewCodeThread.svelte";
+

+
  type ActivityData =
+
    | { kind: "op"; op: FlattenedPatchOperation }
+
    | { kind: "reviewCode"; thread: Thread<CodeLocation> };

  interface Props {
    rid: string;
@@ -63,41 +69,57 @@
    "review.react",
  ]);

-
  const activityItems: ActivityItem<FlattenedPatchOperation>[] = $derived.by(
-
    () => {
-
      const tracker: Partial<Record<Action["type"], Action>> = {};
-
      const items: ActivityItem<FlattenedPatchOperation>[] = [];
-
      activity.forEach(operation => {
-
        operation.actions.forEach((action, actionIndex) => {
-
          if (skippedActivityTypes.has(action.type)) {
-
            tracker[action.type] = action;
-
            return;
-
          }
-
          const previous = tracker[action.type];
-
          // The first `edit` action has nothing to diff against, so the
-
          // renderer skips it. Skip it here too so we don't leave a gap.
-
          if (action.type === "edit" && !previous) {
-
            tracker[action.type] = action;
-
            return;
-
          }
-
          const op: FlattenedPatchOperation = {
-
            ...action,
-
            id: operation.id,
-
            author: operation.author,
-
            timestamp: operation.timestamp,
-
            previous,
-
          };
+
  const activityItems: ActivityItem<ActivityData>[] = $derived.by(() => {
+
    const tracker: Partial<Record<Action["type"], Action>> = {};
+
    const items: ActivityItem<ActivityData>[] = [];
+
    activity.forEach(operation => {
+
      operation.actions.forEach((action, actionIndex) => {
+
        if (skippedActivityTypes.has(action.type)) {
+
          tracker[action.type] = action;
+
          return;
+
        }
+
        const previous = tracker[action.type];
+
        // The first `edit` action has nothing to diff against, so the
+
        // renderer skips it. Skip it here too so we don't leave a gap.
+
        if (action.type === "edit" && !previous) {
          tracker[action.type] = action;
+
          return;
+
        }
+
        const op: FlattenedPatchOperation = {
+
          ...action,
+
          id: operation.id,
+
          author: operation.author,
+
          timestamp: operation.timestamp,
+
          previous,
+
        };
+
        tracker[action.type] = action;
+
        items.push({
+
          key: `${operation.id}:${actionIndex}`,
+
          timestamp: operation.timestamp,
+
          data: { kind: "op", op },
+
        });
+
      });
+
    });
+

+
    (revision.reviews ?? []).forEach(review => {
+
      const reviewComments = review.comments ?? [];
+
      reviewComments
+
        .filter(c => c.location && !c.replyTo && c.id !== review.id)
+
        .forEach(root => {
+
          const replies = reviewComments
+
            .filter(c => c.replyTo === root.id)
+
            .sort((a, b) => a.edits[0].timestamp - b.edits[0].timestamp);
+
          const thread = { root, replies } as Thread<CodeLocation>;
          items.push({
-
            key: `${operation.id}:${actionIndex}`,
-
            timestamp: operation.timestamp,
-
            data: op,
+
            key: `review:${review.id}:${root.id}`,
+
            timestamp: root.edits[0].timestamp,
+
            data: { kind: "reviewCode", thread },
          });
        });
-
      });
-
      return items;
-
    },
-
  );
+
    });
+

+
    return items;
+
  });
  const reviewSummaryFingerprints = $derived(
    new Set(
      (revision.reviews ?? [])
@@ -270,8 +292,18 @@
    </CommentComponent>
  </div>
{:else if view === "activity"}
-
  {#snippet renderActivity(op: FlattenedPatchOperation)}
-
    <PatchActivityItem {op} {patchId} />
+
  {#snippet renderActivity(data: ActivityData)}
+
    {#if data.kind === "op"}
+
      <PatchActivityItem op={data.op} {patchId} />
+
    {:else}
+
      <ReviewCodeThread
+
        {rid}
+
        base={revision.base}
+
        head={revision.head}
+
        thread={data.thread}
+
        {config}
+
        {repoDelegates} />
+
    {/if}
  {/snippet}

  <Discussion