Skip to content

Commit

Permalink
Improve binary detection
Browse files Browse the repository at this point in the history
libgit2 optimizes around not loading the content, when there's no
content callbacks configured.

So we aren't able to get a clear answer from radicle-surf if a file in a
diff is a binary or not.

A simple way without doing additional API requests is to check if the
file type is a executable and if there are no hunks (since we don't get
any deltas in binaries), this is a strong
indicator for a binary.

Signed-off-by: Sebastian Martinez <me@sebastinez.dev>
  • Loading branch information
sebastinez committed Sep 25, 2023
1 parent fb06b5f commit 8929afe
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 5 deletions.
2 changes: 2 additions & 0 deletions httpd-client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
CommitHeader,
Diff,
DiffContent,
DiffFile,
HunkLine,
} from "./lib/project/commit.js";
import type { Issue, IssueState } from "./lib/project/issue.js";
Expand Down Expand Up @@ -44,6 +45,7 @@ export type {
CommitHeader,
Diff,
DiffContent,
DiffFile,
DiffResponse,
HunkLine,
Issue,
Expand Down
15 changes: 14 additions & 1 deletion httpd-client/lib/project/commit.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import type { z } from "zod";
export type { Commits, HunkLine, Commit, Diff, CommitHeader, DiffContent };
export type {
Commits,
HunkLine,
Hunks,
Commit,
Diff,
CommitHeader,
DiffFile,
DiffContent,
};

import { array, literal, number, object, optional, string, union } from "zod";
export { commitHeaderSchema, diffSchema, commitSchema, commitsSchema };
Expand Down Expand Up @@ -36,6 +45,8 @@ const deletionHunkLineSchema = object({
type: literal("deletion"),
});

type DiffFile = z.infer<typeof diffFileSchema>;

const diffFileSchema = object({
oid: string(),
mode: union([
Expand Down Expand Up @@ -64,6 +75,8 @@ const hunkLineSchema = union([
contextHunkLineSchema,
]);

type Hunks = z.infer<typeof changesetHunkSchema>;

const changesetHunkSchema = object({
header: string(),
lines: array(hunkLineSchema),
Expand Down
17 changes: 13 additions & 4 deletions src/views/projects/Changeset.svelte
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script lang="ts">
import type { BaseUrl, Diff } from "@httpd-client";
import type { BaseUrl, Diff, DiffContent, DiffFile } from "@httpd-client";
import { pluralize } from "@app/lib/pluralize";
Expand Down Expand Up @@ -39,6 +39,15 @@
}
return s.join(", ");
};
// Since libgit2 doesn't check binaries thoroughly in diffs,
// and we don't want to loop over all diffs in radicle-surf to get a correct answer.
// We assume that a `blobExecutable` file with no hunks is a binary file.
function getFileType(diff: DiffContent, file: DiffFile): DiffContent["type"] {
return file.mode === "blobExecutable" && diff.hunks.length === 0
? "binary"
: diff.type;
}
</script>

<style>
Expand Down Expand Up @@ -74,7 +83,7 @@
{baseUrl}
{revision}
filePath={file.path}
fileDiff={file.diff}
fileDiff={{ ...file.diff, type: getFileType(file.diff, file.new) }}
headerBadgeCaption="added" />
{/each}
{#each diff.deleted as file}
Expand All @@ -83,7 +92,7 @@
{baseUrl}
{revision}
filePath={file.path}
fileDiff={file.diff}
fileDiff={{ ...file.diff, type: getFileType(file.diff, file.old) }}
headerBadgeCaption="deleted" />
{/each}
{#each diff.modified as file}
Expand All @@ -92,7 +101,7 @@
{baseUrl}
{revision}
filePath={file.path}
fileDiff={file.diff} />
fileDiff={{ ...file.diff, type: getFileType(file.diff, file.new) }} />
{/each}
{#each diff.moved as file}
{#if file.diff}
Expand Down

0 comments on commit 8929afe

Please sign in to comment.