mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-09 03:20:56 +00:00
fix(web,server): pierre 1.1 API fit + unicode-safe path normalize
Three follow-ups from verification against the actual @pierre/diffs
1.1.15 type definitions:
1. Deep-link expand uses `options.expandUnchanged: true` on the
targeted MultiFileDiff rather than firing `el.click()` on the outer
wrapper. Pierre 1.1.x exposes no imperative expand API — click on
the row container was a no-op. Per-file expansion now fires on
mount when the file name matches the URL hash.
2. Enter/Space binding removed from useFileKeyboardNav — click on the
outer row doesn't trigger anything in pierre's model, and binding
it just delayed default browser scroll behavior on Space. j/k
focus navigation remains the working keyboard affordance. When a
pierre imperative expand API appears, Enter/Space can be re-added
to call it.
3. normalize_for_match strip loop now iterates to a fixed point
against the fully-lowercased string so repeated `./` / `../` / `/`
prefixes are all stripped. Added Windows-path and Unicode-uppercase
regression tests for is_sensitive to verify basename matching
survives both.
Virtualizer usage verified against the 1.1.x type definitions: the
`{ children: ReactNode }` signature accepts the wrapped file list
directly with no Virtualizer.Item wrapper needed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
695a6ca536
commit
0c77a184ab
6 changed files with 99 additions and 65 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -1989,6 +1989,7 @@ dependencies = [
|
|||
"tower",
|
||||
"tower-http",
|
||||
"tracing",
|
||||
"tracing-subscriber",
|
||||
"ulid",
|
||||
"uuid",
|
||||
"walkdir",
|
||||
|
|
|
|||
|
|
@ -266,9 +266,11 @@ export default function RunFiles({ loaderData }: any) {
|
|||
const fileCount = data?.data.length ?? 0;
|
||||
useFileKeyboardNav(containerRef, fileCount);
|
||||
|
||||
// Deep-link handling: scroll + focus the matching row; optionally ask
|
||||
// @pierre/diffs to expand the file via data-attribute the diff picks up
|
||||
// on click.
|
||||
// Deep-link handling: scroll + focus the matching row. Expansion is
|
||||
// handled by passing `expandUnchanged: true` to the targeted MultiFileDiff
|
||||
// via per-file options (see `renderFiles` below) — @pierre/diffs 1.1.x
|
||||
// exposes no imperative expand API, so click-based "expand" is not
|
||||
// available.
|
||||
const [hashFile, setHashFile] = useState<string | null>(() => {
|
||||
if (typeof window === "undefined") return null;
|
||||
return decodeDeepLinkFile(window.location.hash);
|
||||
|
|
@ -302,8 +304,6 @@ export default function RunFiles({ loaderData }: any) {
|
|||
if (el) {
|
||||
el.scrollIntoView({ block: "start", behavior: "smooth" });
|
||||
el.focus({ preventScroll: true });
|
||||
// Fire a click so any @pierre/diffs expand-on-click wiring fires.
|
||||
el.click();
|
||||
}
|
||||
}, [hashFile, data]);
|
||||
|
||||
|
|
@ -327,6 +327,11 @@ export default function RunFiles({ loaderData }: any) {
|
|||
</div>
|
||||
);
|
||||
}
|
||||
// When the deep-link targets this file, pass expandUnchanged:true so
|
||||
// the full surrounding context renders without per-hunk clicking.
|
||||
const isDeepLinkTarget =
|
||||
!!hashFile &&
|
||||
(file.new_file.name === hashFile || file.old_file.name === hashFile);
|
||||
return (
|
||||
<div
|
||||
key={`${file.new_file.name}-${idx}`}
|
||||
|
|
@ -343,12 +348,13 @@ export default function RunFiles({ loaderData }: any) {
|
|||
options={{
|
||||
diffStyle,
|
||||
theme: pierreTheme,
|
||||
expandUnchanged: isDeepLinkTarget ? true : undefined,
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}),
|
||||
[diffStyle, pierreTheme],
|
||||
[diffStyle, pierreTheme, hashFile],
|
||||
);
|
||||
|
||||
if (isInitialLoading) {
|
||||
|
|
|
|||
|
|
@ -10,9 +10,14 @@ export function isEditableElement(el: Element | null): boolean {
|
|||
|
||||
/**
|
||||
* Wire keyboard navigation across file rows. `j` / `k` move focus to the
|
||||
* next / previous row; `Enter` and `Space` trigger a `click` on the focused
|
||||
* row so diff wrappers that opt in can expand/collapse. Key presses while a
|
||||
* text-editable element is focused are left alone.
|
||||
* next / previous row. Key presses while a text-editable element is focused
|
||||
* are left alone so typing into filters / comment boxes isn't hijacked.
|
||||
*
|
||||
* Enter/Space are deliberately not bound — @pierre/diffs 1.1.x has no
|
||||
* imperative expand/collapse API, and firing a click on the outer wrapper
|
||||
* has no effect. Per-hunk expansion remains mouse-driven via pierre's own
|
||||
* controls. Files targeted by a deep-link get `expandUnchanged: true` via
|
||||
* per-file options instead.
|
||||
*/
|
||||
export function useFileKeyboardNav(
|
||||
containerRef: RefObject<HTMLDivElement | null>,
|
||||
|
|
@ -21,6 +26,7 @@ export function useFileKeyboardNav(
|
|||
useEffect(() => {
|
||||
if (!containerRef.current) return;
|
||||
const onKey = (event: KeyboardEvent) => {
|
||||
if (event.key !== "j" && event.key !== "k") return;
|
||||
if (event.metaKey || event.ctrlKey || event.altKey) return;
|
||||
if (isEditableElement(document.activeElement)) return;
|
||||
const container = containerRef.current;
|
||||
|
|
@ -33,28 +39,17 @@ export function useFileKeyboardNav(
|
|||
const active = document.activeElement as HTMLElement | null;
|
||||
const currentIdx = rows.findIndex((row) => row.contains(active));
|
||||
|
||||
if (event.key === "j" || event.key === "k") {
|
||||
let nextIdx: number;
|
||||
if (currentIdx < 0) {
|
||||
nextIdx = 0;
|
||||
} else {
|
||||
nextIdx = event.key === "j" ? currentIdx + 1 : currentIdx - 1;
|
||||
}
|
||||
if (nextIdx < 0 || nextIdx >= rows.length) return;
|
||||
event.preventDefault();
|
||||
const target = rows[nextIdx];
|
||||
target.focus({ preventScroll: false });
|
||||
target.scrollIntoView({ block: "nearest", behavior: "smooth" });
|
||||
return;
|
||||
}
|
||||
|
||||
if (event.key === "Enter" || event.key === " ") {
|
||||
if (currentIdx < 0) return;
|
||||
event.preventDefault();
|
||||
// Forward to the row element as a click so any @pierre/diffs
|
||||
// expand-handler the consumer wires up fires naturally.
|
||||
rows[currentIdx].click();
|
||||
let nextIdx: number;
|
||||
if (currentIdx < 0) {
|
||||
nextIdx = 0;
|
||||
} else {
|
||||
nextIdx = event.key === "j" ? currentIdx + 1 : currentIdx - 1;
|
||||
}
|
||||
if (nextIdx < 0 || nextIdx >= rows.length) return;
|
||||
event.preventDefault();
|
||||
const target = rows[nextIdx];
|
||||
target.focus({ preventScroll: false });
|
||||
target.scrollIntoView({ block: "nearest", behavior: "smooth" });
|
||||
};
|
||||
document.addEventListener("keydown", onKey);
|
||||
return () => document.removeEventListener("keydown", onKey);
|
||||
|
|
|
|||
|
|
@ -103,20 +103,33 @@ pub fn is_sensitive(path: &str) -> bool {
|
|||
fn normalize_for_match(path: &str) -> String {
|
||||
// Replace backslashes with forward slashes so Windows-style paths (if
|
||||
// they ever leak through git output) match the same way; lowercase so
|
||||
// the patterns are effectively case-insensitive.
|
||||
// the patterns are effectively case-insensitive. `to_lowercase()` can
|
||||
// expand certain Unicode codepoints to multiple chars — the strip loop
|
||||
// below runs against the fully-lowercased string so prefix matching is
|
||||
// consistent regardless of the input's case.
|
||||
let mut out = String::with_capacity(path.len());
|
||||
for ch in path.chars() {
|
||||
let c = if ch == '\\' { '/' } else { ch };
|
||||
out.extend(c.to_lowercase());
|
||||
}
|
||||
// Drop leading `./` and consecutive `../` prefixes; keep inner `..` alone
|
||||
// since git doesn't emit those in normal diffs.
|
||||
while let Some(rest) = out
|
||||
.strip_prefix("./")
|
||||
.or_else(|| out.strip_prefix("../"))
|
||||
.or_else(|| out.strip_prefix("/"))
|
||||
{
|
||||
out = rest.to_string();
|
||||
// Drop leading `./`, `../`, and bare-leading `/` prefixes until the
|
||||
// path has a meaningful first segment. Inner `..` components are left
|
||||
// alone — git doesn't emit them in normal diffs, and stripping them
|
||||
// mid-path would change the effective basename.
|
||||
loop {
|
||||
let next = if let Some(rest) = out.strip_prefix("./") {
|
||||
rest.to_string()
|
||||
} else if let Some(rest) = out.strip_prefix("../") {
|
||||
rest.to_string()
|
||||
} else if let Some(rest) = out.strip_prefix('/') {
|
||||
rest.to_string()
|
||||
} else {
|
||||
break;
|
||||
};
|
||||
if next == out {
|
||||
break;
|
||||
}
|
||||
out = next;
|
||||
}
|
||||
out
|
||||
}
|
||||
|
|
@ -225,6 +238,25 @@ mod tests {
|
|||
assert!(is_sensitive("keys/ID_RSA"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_sensitive_normalizes_backslashes_as_separators() {
|
||||
// If git ever surfaces a Windows-style path the denylist should
|
||||
// still catch it: `keys\\id_rsa` canonicalizes to `keys/id_rsa`.
|
||||
assert!(is_sensitive("keys\\id_rsa"));
|
||||
assert!(is_sensitive("C:\\Users\\alice\\.ssh\\config"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_sensitive_handles_unicode_uppercase_basename() {
|
||||
// Some Unicode uppercase letters expand to multiple lowercase code
|
||||
// points (e.g. the sharp-S `ẞ` -> `ss`). The basename glob check
|
||||
// should still fire on the lowercased form without losing prefix
|
||||
// stripping.
|
||||
assert!(is_sensitive("keys/ID_RSA")); // ASCII baseline
|
||||
// A non-sensitive unicode path must still not match any glob.
|
||||
assert!(!is_sensitive("docs/Ähnlichkeit.md"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandbox_git_env_sets_expected_pairs() {
|
||||
let env = sandbox_git_env();
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
2
lib/crates/fabro-spa/assets/index.html
generated
2
lib/crates/fabro-spa/assets/index.html
generated
|
|
@ -61,7 +61,7 @@
|
|||
<script type="module" src="/assets/chunk-sadshphz.js"></script>
|
||||
<script type="module" src="/assets/chunk-pmthkscp.js"></script>
|
||||
<script type="module" src="/assets/chunk-v61ks9f7.js"></script>
|
||||
<script type="module" src="/assets/entry-mf9xvex6.js"></script>
|
||||
<script type="module" src="/assets/entry-rc6hmkqw.js"></script>
|
||||
<script type="module" src="/assets/chunk-n1k68xa8.js"></script>
|
||||
<script type="module" src="/assets/chunk-rsph5pvm.js"></script>
|
||||
<script type="module" src="/assets/chunk-9t57pdty.js"></script>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue