Skip to content
Merged
8 changes: 5 additions & 3 deletions apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { parsePullRequestQuery } from "@t3tools/client-runtime/pull-request-query";
import { LegendList, type LegendListRef } from "@legendapp/list/react-native";
import { useNavigation } from "@react-navigation/native";
import { use, useCallback, useEffect, useMemo, useRef, useState } from "react";
Expand Down Expand Up @@ -81,6 +82,7 @@ export function PullRequestsScreen(
const debouncedQuery = useDebouncedValue(tab.query.trim(), 180);
// Dismissal must restore the cached unfiltered list before restoring its offset.
const query = tab.searchMode === "resting" ? "" : debouncedQuery;
const parsedQuery = useMemo(() => parsePullRequestQuery(query), [query]);
const listControls = usePullRequestListControls();
const selectedEnvironmentId = listControls.environmentId;
const environmentId = environments.some((item) => item.environmentId === selectedEnvironmentId)
Expand Down Expand Up @@ -126,13 +128,13 @@ export function PullRequestsScreen(
input: {
state: controls.state,
involvement: controls.involvement,
filters: controls.filters,
filters: { ...controls.filters, ...parsedQuery.filters },
limit: PR_LIST_PAGE_SIZE,
...(controls.projectId ? { projectId: ProjectId.make(controls.projectId) } : {}),
...(query ? { query } : {}),
...(parsedQuery.text ? { query: parsedQuery.text } : {}),
},
})),
[environments, environmentId, configs, controls, query],
[environments, environmentId, configs, controls, parsedQuery],
);
const result = usePullRequestList(targets);
const connectedTargets = targets.filter((target) =>
Expand Down
31 changes: 31 additions & 0 deletions apps/mobile/src/features/pull-requests/pullRequestList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,37 @@ describe("recycled PR list rows", () => {
});
});

describe("mobile PR draft queries", () => {
const rows = [entry(1), entry(2, { isDraft: true })];

it.each([
["draft:false", [1]],
["draft:true", [2]],
["DRAFT:FALSE", [1]],
["", [2, 1]],
])("filters %s on local and host search", (query, expected) => {
for (const localSearchHosts of [new Set<string>(), new Set(["github.com"])]) {
expect(
filterPullRequestList(rows, DEFAULT_PR_LIST_CONTROLS, {}, query, localSearchHosts).map(
(row) => row.number,
),
).toEqual(expected);
}
});

it("lets the typed draft qualifier override the menu and keeps free text", () => {
expect(
filterPullRequestList(
rows,
{ ...DEFAULT_PR_LIST_CONTROLS, filters: { draft: "only" } },
{},
"draft:false Change 1",
new Set(["github.com"]),
).map((row) => row.number),
).toEqual([1]);
});
});

describe("mobile PR list pagination", () => {
it("continues only the repositories that have more, preserving the query and filters", () => {
const first = pullRequestListHead(
Expand Down
6 changes: 4 additions & 2 deletions apps/mobile/src/features/pull-requests/pullRequestList.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { parsePullRequestQuery } from "@t3tools/client-runtime/pull-request-query";
import type {
EnvironmentId,
PullRequestListEntry,
Expand Down Expand Up @@ -189,8 +190,9 @@ export function filterPullRequestList<
connectedEnvironmentIds?: ReadonlySet<string>,
) {
const normalize = (value: string) => value.toLowerCase();
const words = query.toLowerCase().split(/\s+/).filter(Boolean);
const { filters } = controls;
const parsed = parsePullRequestQuery(query);
const words = parsed.text.toLowerCase().split(/\s+/).filter(Boolean);
const filters = { ...controls.filters, ...parsed.filters };
const filtered = entries.filter((entry) => {
if (controls.state !== "all" && entry.state !== controls.state) return false;
if (controls.projectId && entry.projectId !== controls.projectId) return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { parsePullRequestQuery } from "@t3tools/client-runtime/pull-request-query";
import { SourceControlProviderKind } from "@t3tools/contracts";
import type {
EnvironmentId,
Expand All @@ -18,7 +19,6 @@ import {
groupPullRequestsByInvolvement,
matchesPullRequestFilters,
matchesPullRequestQuery,
parsePullRequestQuery,
pullRequestStatsBatches,
pullRequestStatsKeysToRequest,
pullRequestStatsRefreshBatches,
Expand Down
152 changes: 0 additions & 152 deletions apps/web/src/components/pullRequest/pullRequestList.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,158 +159,6 @@ function isAuthoredByViewer(entry: ScopedEntry, viewers: PullRequestViewers): bo
return viewer !== null && normalize(entry.author?.login) === viewer;
}

/** What `review:` and `status:` take, in GitHub's spelling and in the contract's. */
const REVIEW_VALUES: Record<string, PullRequestListFilters["review"]> = {
approved: "approved",
changes_requested: "changes-requested",
"changes-requested": "changes-requested",
required: "review-required",
"review-required": "review-required",
none: "none",
};
const CHECKS_VALUES: Record<string, PullRequestListFilters["checks"]> = {
success: "passing",
passing: "passing",
failure: "failing",
failing: "failing",
};

/**
* One token of a typed query: a run of non-space characters in which a quoted stretch counts as
* part of the token, so `label:"needs design"` stays whole. An unbalanced quote is dropped rather
* than swallowing the rest of the line.
*/
const QUERY_TOKEN = /(?:[^\s"]|"[^"]*")+/g;
/** The contract's own ceilings on a qualifier: this many names, each this long. */
const MAX_QUALIFIER_VALUES = 10;
const MAX_QUALIFIER_LENGTH = 200;

function qualifierValue(raw: string): string {
return raw.replaceAll('"', "").trim();
}

/** A qualifier's value as the list it may be: split on commas, each name unquoted on its own. */
function splitQualifierList(raw: string): string[] {
// Quoting names one whole label however many commas it holds: `label:"needs,triage"` asks for
// the label written that way, not for either half of it.
if (/^\s*"[^"]*"\s*$/.test(raw)) {
const whole = qualifierValue(raw);
return whole.length === 0 ? [] : [whole];
}
return raw
.split(",")
.map((part) => qualifierValue(part))
.filter((part) => part.length > 0);
}

/**
* A typed query is written by hand, and the contract bounds what a qualifier may carry — so what
* is typed past those bounds is cut here rather than refused by the request that carries it.
*/
function boundedNames(names: ReadonlyArray<string>): string[] {
return names
.slice(0, MAX_QUALIFIER_VALUES)
.map((name) => name.slice(0, MAX_QUALIFIER_LENGTH).trim())
.filter((name) => name.length > 0);
}

/**
* A typed query split into the qualifiers the hosts can act on and the text that is left. Written
* GitHub's way — `label:foo`, `-label:"needs design"`, `author:octocat`, `draft:true`,
* `review:approved`, `status:success` — because a project's labels are its own and a menu cannot
* list them.
*
* A key this does not know is read as a label of that whole name: repositories namespace their
* labels with a colon — `size:XXL`, `area:web`, `vouch:trusted` — and someone typing one means
* the label, not a description that happens to contain it. `-size:XXL` excludes the same way.
*
* Quoting is the way back to plain text: `"size:XXL"` is searched for as written, which is what
* makes a literal search for a colon still possible. A known key whose value it does not take is
* text too, so a search for "status:" itself is still findable.
*/
export function parsePullRequestQuery(raw: string): {
readonly text: string;
readonly filters: PullRequestListFilters;
} {
const text: string[] = [];
const labels: string[][] = [];
const excludedLabels: string[] = [];
let author: string | undefined;
let draft: PullRequestListFilters["draft"];
let review: PullRequestListFilters["review"];
let checks: PullRequestListFilters["checks"];
for (const [token] of raw.matchAll(QUERY_TOKEN)) {
const qualifier = /^(-?)([A-Za-z][A-Za-z0-9_-]*):(.*)$/.exec(token);
const value = qualifier === null ? "" : qualifierValue(qualifier[3] ?? "");
const negated = qualifier?.[1] === "-";
switch (value.length === 0 ? "" : (qualifier?.[2]?.toLowerCase() ?? "")) {
case "label": {
// GitHub's own OR: `label:a,b` is one qualifier satisfied by either name. Negated, the
// comma excludes each — a row carrying any of them goes.
const names = boundedNames(splitQualifierList(qualifier?.[3] ?? ""));
if (names.length === 0) break;
if (negated) excludedLabels.push(...names);
else labels.push(names);
continue;
}
case "author":
if (negated) break;
author = value.slice(0, MAX_QUALIFIER_LENGTH).trim();
continue;
case "draft":
if (negated || (value.toLowerCase() !== "true" && value.toLowerCase() !== "false")) break;
draft = value.toLowerCase() === "true" ? "only" : "hide";
continue;
case "review": {
const decision = negated ? undefined : REVIEW_VALUES[value.toLowerCase()];
if (decision === undefined) break;
review = decision;
continue;
}
case "status":
case "checks": {
const state = negated ? undefined : CHECKS_VALUES[value.toLowerCase()];
if (state === undefined) break;
checks = state;
continue;
}
case "":
break;
default:
// An unknown key, read as the namespaced label it almost always is. A pasted link is
// not one — `https://…` would otherwise become a label named after its own scheme.
if (!value.startsWith("/")) {
// The key names the namespace, so the bare parts of `size:S,XS` are both sizes. A part
// that already carries a colon names its whole label — `size:S,size:XS` is the same
// pair written out, and prefixing it again would ask for `size:size:XS`.
const names = boundedNames(
splitQualifierList(qualifier?.[3] ?? "").map((name) =>
name.includes(":") ? name : `${qualifier?.[2] ?? ""}:${name}`,
),
);
if (names.length === 0) break;
if (negated) excludedLabels.push(...names);
else labels.push(names);
continue;
}
}
text.push(token);
}
return {
text: text.join(" "),
filters: {
...(labels.length === 0 ? {} : { labels: labels.slice(0, MAX_QUALIFIER_VALUES) }),
...(excludedLabels.length === 0
? {}
: { excludedLabels: excludedLabels.slice(0, MAX_QUALIFIER_VALUES) }),
...(author === undefined ? {} : { author }),
...(draft === undefined ? {} : { draft }),
...(review === undefined ? {} : { review }),
...(checks === undefined ? {} : { checks }),
},
};
}

/** Free-text filter over the fields a row actually shows, plus `#123` / `123`. */
export function matchesPullRequestQuery(entry: PullRequestListEntry, query: string): boolean {
const normalizedQuery = query.trim().toLowerCase();
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/routes/_chat.pull-requests.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { parsePullRequestQuery } from "@t3tools/client-runtime/pull-request-query";
import { RefreshIcon } from "~/components/ui/refresh-icon";
import { Spinner } from "~/components/ui/spinner";
import { useShortcutModifierState } from "~/shortcutModifierState";
Expand Down Expand Up @@ -58,7 +59,6 @@ import {
groupPullRequestsByInvolvement,
matchesPullRequestFilters,
matchesPullRequestQuery,
parsePullRequestQuery,
narrowPullRequestsToFilters,
mergePullRequestDiffStats,
partitionPullRequestsWithPriority,
Expand Down
4 changes: 4 additions & 0 deletions packages/client-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
"private": true,
"type": "module",
"exports": {
"./pull-request-query": {
"types": "./src/pullRequestQuery.ts",
"default": "./src/pullRequestQuery.ts"
},
"./composerThreadItems": {
"types": "./src/composerThreadItems.ts",
"default": "./src/composerThreadItems.ts"
Expand Down
16 changes: 16 additions & 0 deletions packages/client-runtime/src/pullRequestQuery.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { describe, expect, it } from "vite-plus/test";
import { parsePullRequestQuery } from "./pullRequestQuery.ts";

describe("quoted commas in PR qualifier lists", () => {
it.each([
['label:"needs,triage",wip', { labels: [["needs,triage", "wip"]] }],
[
'label:wip,"needs,triage","needs design"',
{ labels: [["wip", "needs,triage", "needs design"]] },
],
['-label:"needs,triage",wip', { excludedLabels: ["needs,triage", "wip"] }],
['area:"web,mobile",server', { labels: [["area:web,mobile", "area:server"]] }],
])("keeps quoted names whole in %s", (query, filters) => {
expect(parsePullRequestQuery(query)).toEqual({ text: "", filters });
});
});
Loading
Loading