diff --git a/crates/store/src/lib.rs b/crates/store/src/lib.rs index a205108..4817023 100644 --- a/crates/store/src/lib.rs +++ b/crates/store/src/lib.rs @@ -1470,10 +1470,7 @@ mod tests { let location_dir = TempDir::new().unwrap(); let location_path = location_dir.path().to_path_buf(); - let settings = UiLayoutSettings { - create_readme_in_new_locations: false, - ..UiLayoutSettings::default() - }; + let settings = UiLayoutSettings { create_readme_in_new_locations: false, ..UiLayoutSettings::default() }; store.ui_layout_set(&settings).unwrap(); let location = store @@ -1493,10 +1490,7 @@ mod tests { let location_dir = TempDir::new().unwrap(); let location_path = location_dir.path().to_path_buf(); - let settings = UiLayoutSettings { - create_readme_in_new_locations: false, - ..UiLayoutSettings::default() - }; + let settings = UiLayoutSettings { create_readme_in_new_locations: false, ..UiLayoutSettings::default() }; store.ui_layout_set(&settings).unwrap(); let location = store @@ -1518,10 +1512,7 @@ mod tests { let location_dir = TempDir::new().unwrap(); let location_path = location_dir.path().to_path_buf(); - let settings = UiLayoutSettings { - create_readme_in_new_locations: false, - ..UiLayoutSettings::default() - }; + let settings = UiLayoutSettings { create_readme_in_new_locations: false, ..UiLayoutSettings::default() }; store.ui_layout_set(&settings).unwrap(); let location = store @@ -1650,10 +1641,7 @@ mod tests { let location_dir = TempDir::new().unwrap(); let location_path = location_dir.path().to_path_buf(); - let settings = UiLayoutSettings { - create_readme_in_new_locations: false, - ..UiLayoutSettings::default() - }; + let settings = UiLayoutSettings { create_readme_in_new_locations: false, ..UiLayoutSettings::default() }; store.ui_layout_set(&settings).unwrap(); let location = store @@ -1771,6 +1759,59 @@ mod tests { assert_eq!(loaded.focus_dimming_mode, FocusDimmingMode::Sentence); } + #[test] + fn test_style_check_settings_defaults() { + let (store, _temp) = create_test_store(); + let settings = store.style_check_get().unwrap(); + + assert_eq!(settings, StyleCheckSettings::default()); + assert_eq!(settings.marker_style, settings::StyleMarkerStyle::Highlight); + } + + #[test] + fn test_style_check_settings_round_trip() { + let (store, _temp) = create_test_store(); + let settings = StyleCheckSettings { + enabled: true, + categories: settings::StyleCheckCategorySettings { filler: true, redundancy: false, cliche: true }, + custom_patterns: vec![StyleCheckPattern { + text: "in this day and age".to_string(), + category: "cliche".to_string(), + replacement: Some("today".to_string()), + }], + marker_style: settings::StyleMarkerStyle::Underline, + }; + + store.style_check_set(&settings).unwrap(); + let loaded = store.style_check_get().unwrap(); + + assert_eq!(loaded, settings); + } + + #[test] + fn test_style_check_settings_backfills_marker_style() { + let (store, _temp) = create_test_store(); + let conn = store + .conn + .lock() + .expect("expected to lock database connection for test"); + + conn.execute( + "INSERT INTO app_settings (key, value, updated_at) VALUES (?1, ?2, ?3)", + params![ + STYLE_CHECK_SETTINGS_KEY, + "{\"enabled\":true,\"categories\":{\"filler\":true,\"redundancy\":true,\"cliche\":true},\"custom_patterns\":[]}", + Utc::now().to_rfc3339(), + ], + ) + .unwrap(); + drop(conn); + + let loaded = store.style_check_get().unwrap(); + assert!(loaded.enabled); + assert_eq!(loaded.marker_style, settings::StyleMarkerStyle::Highlight); + } + #[test] fn test_global_capture_settings_defaults() { let (store, _temp) = create_test_store(); diff --git a/crates/store/src/settings.rs b/crates/store/src/settings.rs index d40df85..216236b 100644 --- a/crates/store/src/settings.rs +++ b/crates/store/src/settings.rs @@ -37,6 +37,10 @@ fn default_create_readme_in_new_locations() -> bool { true } +fn default_style_marker_style() -> StyleMarkerStyle { + StyleMarkerStyle::default() +} + #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)] #[serde(rename_all = "lowercase")] pub enum FocusDimmingMode { @@ -53,6 +57,15 @@ pub struct StyleCheckCategorySettings { pub cliche: bool, } +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)] +#[serde(rename_all = "lowercase")] +pub enum StyleMarkerStyle { + #[default] + Highlight, + Strikethrough, + Underline, +} + impl Default for StyleCheckCategorySettings { fn default() -> Self { Self { filler: true, redundancy: true, cliche: true } @@ -67,6 +80,8 @@ pub struct StyleCheckSettings { pub categories: StyleCheckCategorySettings, #[serde(default)] pub custom_patterns: Vec, + #[serde(default = "default_style_marker_style")] + pub marker_style: StyleMarkerStyle, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] diff --git a/fixtures/markdown/style-check-sample.md b/fixtures/markdown/style-check-sample.md new file mode 100644 index 0000000..cf611dd --- /dev/null +++ b/fixtures/markdown/style-check-sample.md @@ -0,0 +1,12 @@ +# Style Check Fixture + +This paragraph is basically a test fixture. +We acted in order to finish quickly due to the fact that time was short. +When discussions dragged, we started to beat around the bush. + +## Expected style flags + +- `basically` (filler) +- `in order to` (redundancy) +- `due to the fact that` (redundancy) +- `beat around the bush` (cliche) diff --git a/justfile b/justfile index ef737cd..39ddf74 100644 --- a/justfile +++ b/justfile @@ -13,7 +13,7 @@ compile: # Overall code quality check check: format lint compile test -# Finds comments +# Finds comments in rust code find-comments: rg -n --pcre2 '^\s*//(?![!/])' -g '*.rs' diff --git a/src/__tests__/AppHeaderBar.test.tsx b/src/__tests__/AppHeaderBar.test.tsx index ac3f33b..aaf9dcc 100644 --- a/src/__tests__/AppHeaderBar.test.tsx +++ b/src/__tests__/AppHeaderBar.test.tsx @@ -1,11 +1,14 @@ import { AppHeaderBar } from "$components/layout/AppHeaderBar"; import { useViewportTier } from "$hooks/useViewportTier"; -import { useAppHeaderBarState, useHelpSheetState } from "$state/selectors"; +import { useAppHeaderBarState, useHelpSheetState, useStyleDiagnosticsUiState } from "$state/selectors"; import { fireEvent, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("$hooks/useViewportTier", () => ({ useViewportTier: vi.fn() })); -vi.mock("$state/selectors", () => ({ useAppHeaderBarState: vi.fn(), useHelpSheetState: vi.fn() })); +vi.mock( + "$state/selectors", + () => ({ useAppHeaderBarState: vi.fn(), useHelpSheetState: vi.fn(), useStyleDiagnosticsUiState: vi.fn() }), +); describe("AppHeaderBar", () => { beforeEach(() => { @@ -21,6 +24,7 @@ describe("AppHeaderBar", () => { setShowSearch: vi.fn(), }); vi.mocked(useHelpSheetState).mockReturnValue({ isOpen: false, setOpen: vi.fn(), toggle: vi.fn() }); + vi.mocked(useStyleDiagnosticsUiState).mockReturnValue({ isOpen: false, setOpen: vi.fn(), toggle: vi.fn() }); vi.mocked(useViewportTier).mockReturnValue({ viewportWidth: 1280, tier: "standard", @@ -40,4 +44,18 @@ describe("AppHeaderBar", () => { expect(setHelpSheetOpen).toHaveBeenCalledWith(true); }); + + it("toggles style diagnostics from the header action", () => { + const toggleStyleDiagnostics = vi.fn(); + vi.mocked(useStyleDiagnosticsUiState).mockReturnValue({ + isOpen: false, + setOpen: vi.fn(), + toggle: toggleStyleDiagnostics, + }); + + render(); + fireEvent.click(screen.getByTitle("Show style diagnostics")); + + expect(toggleStyleDiagnostics).toHaveBeenCalledOnce(); + }); }); diff --git a/src/__tests__/WorkspacePanel.test.tsx b/src/__tests__/WorkspacePanel.test.tsx index 57f43b0..18d13f5 100644 --- a/src/__tests__/WorkspacePanel.test.tsx +++ b/src/__tests__/WorkspacePanel.test.tsx @@ -57,6 +57,7 @@ type WorkspacePanelPropOverrides = { editor?: Partial; preview?: Partial; statusBar?: Partial; + diagnostics?: Partial; }; const createSidebarState = (overrides: Partial = {}): SidebarStateReturn => ({ @@ -102,6 +103,7 @@ const createEditorPresentationState = ( enabled: false, categories: { filler: true, redundancy: true, cliche: true }, customPatterns: [], + markerStyle: "highlight", }, ...overrides, }); @@ -181,6 +183,7 @@ const createWorkspacePanelProps = (overrides: WorkspacePanelPropOverrides = {}): ...overrides.preview, }, statusBar: { stats: { cursorLine: 1, cursorColumn: 1, wordCount: 0, charCount: 0 }, ...overrides.statusBar }, + diagnostics: { isVisible: false, matches: [], onSelectMatch: vi.fn(), onClose: vi.fn(), ...overrides.diagnostics }, }); const renderWorkspacePanel = ( @@ -267,4 +270,29 @@ describe("WorkspacePanel", () => { globalThis.dispatchEvent(new Event("resize")); }); }); + + it("renders diagnostics panel and routes match actions", () => { + const onSelectMatch = vi.fn(); + const onClose = vi.fn(); + const match = { + from: 6, + to: 15, + text: "basically", + category: "filler" as const, + replacement: "remove", + line: 1, + column: 6, + }; + + renderWorkspacePanel({ diagnostics: { isVisible: true, matches: [match], onSelectMatch, onClose } }, { + workspacePanelSidebarState: { sidebarCollapsed: false }, + }); + + expect(screen.getByText("Style Check")).toBeInTheDocument(); + fireEvent.click(screen.getByText("basically")); + expect(onSelectMatch).toHaveBeenCalledWith(match); + + fireEvent.click(screen.getByLabelText("Close diagnostics panel")); + expect(onClose).toHaveBeenCalledOnce(); + }); }); diff --git a/src/__tests__/pattern-matcher.test.ts b/src/__tests__/pattern-matcher.test.ts index 7191451..65c69e1 100644 --- a/src/__tests__/pattern-matcher.test.ts +++ b/src/__tests__/pattern-matcher.test.ts @@ -42,6 +42,18 @@ describe("PatternMatcher", () => { expect(matches[0].end).toBe(12); }); + it("should respect unicode word boundaries", () => { + const patterns = [{ text: "just", category: "filler" as const }]; + const matcher = new PatternMatcher(patterns); + const text = "éjust should not match, but just should."; + const matches = matcher.scan(text); + const expectedStart = text.lastIndexOf("just"); + + expect(matches).toHaveLength(1); + expect(matches[0].start).toBe(expectedStart); + expect(matches[0].end).toBe(matches[0].start + 4); + }); + it("should be case-insensitive", () => { const patterns = [{ text: "basically", category: "filler" as const }]; const matcher = new PatternMatcher(patterns); diff --git a/src/__tests__/style-check.test.ts b/src/__tests__/style-check.test.ts new file mode 100644 index 0000000..46dcfc6 --- /dev/null +++ b/src/__tests__/style-check.test.ts @@ -0,0 +1,154 @@ +import { PatternMatcher } from "$editor/pattern-matcher"; +import { collectStyleMatches, resolveStyleMatchAtPosition, styleCheck, type StyleMatch } from "$editor/style-check"; +import { EditorState, Text } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { describe, expect, it, vi } from "vitest"; + +function lineAndColumn(text: string, position: number): { line: number; column: number } { + const lines = text.slice(0, position).split("\n"); + return { line: lines.length, column: lines.at(-1)?.length ?? 0 }; +} + +describe("styleCheck", () => { + it("collects exact absolute ranges with document-cased text", () => { + const doc = "Start BASICALLY now.\nAnd at this point in time we decide."; + const matcher = new PatternMatcher([{ text: "basically", category: "filler" }, { + text: "at this point in time", + category: "redundancy", + replacement: "now", + }]); + + const matches = collectStyleMatches(Text.of(doc.split("\n")), matcher); + const firstFrom = doc.indexOf("BASICALLY"); + const secondFrom = doc.indexOf("at this point in time"); + const first = lineAndColumn(doc, firstFrom); + const second = lineAndColumn(doc, secondFrom); + + expect(matches).toStrictEqual([{ + from: firstFrom, + to: firstFrom + "BASICALLY".length, + text: "BASICALLY", + category: "filler", + replacement: undefined, + line: first.line, + column: first.column, + }, { + from: secondFrom, + to: secondFrom + "at this point in time".length, + text: "at this point in time", + category: "redundancy", + replacement: "now", + line: second.line, + column: second.column, + }]); + }); + + it("deduplicates identical overlapping dictionary/custom results", () => { + const matcher = new PatternMatcher([{ text: "actually", category: "filler" }, { + text: "actually", + category: "filler", + }]); + + const matches = collectStyleMatches(Text.of(["actually"]), matcher); + expect(matches).toHaveLength(1); + expect(matches[0]).toMatchObject({ from: 0, to: 8, text: "actually", category: "filler" }); + }); + + it("resolves tooltip hover positions with side-aware boundaries", () => { + const matches: StyleMatch[] = [{ from: 5, to: 10, text: "match", category: "filler", line: 1, column: 5 }]; + + expect(resolveStyleMatchAtPosition(matches, 7, 1)).toStrictEqual(matches[0]); + expect(resolveStyleMatchAtPosition(matches, 10, -1)).toStrictEqual(matches[0]); + expect(resolveStyleMatchAtPosition(matches, 10, 1)).toBeNull(); + }); + + it("emits style matches through the editor extension for full-document ranges", () => { + const onMatchesChange = vi.fn(); + const state = EditorState.create({ + doc: "Prefix.\nThis is BASICALLY fine.\nLater, at this point in time we ship.", + extensions: [ + styleCheck({ + enabled: true, + categories: { filler: false, redundancy: false, cliche: false }, + customPatterns: [{ text: "basically", category: "filler" }, { + text: "at this point in time", + category: "redundancy", + replacement: "now", + }], + onMatchesChange, + }), + ], + }); + const view = new EditorView({ state }); + + expect(onMatchesChange).toHaveBeenCalled(); + const initialMatches = onMatchesChange.mock.lastCall?.[0] as StyleMatch[]; + expect(initialMatches).toHaveLength(2); + expect(initialMatches[0]).toMatchObject({ text: "BASICALLY", category: "filler" }); + expect(initialMatches[1]).toMatchObject({ + text: "at this point in time", + category: "redundancy", + replacement: "now", + }); + + view.dispatch({ changes: { from: 0, to: 0, insert: "Actually. " } }); + const updatedMatches = onMatchesChange.mock.lastCall?.[0] as StyleMatch[]; + expect(updatedMatches).toHaveLength(2); + + view.destroy(); + }); + + it("loads built-in dictionaries and reports filler, redundancy, and cliche matches", () => { + const onMatchesChange = vi.fn(); + const state = EditorState.create({ + doc: "Basically we act in order to ship, and we may beat around the bush.", + extensions: [ + styleCheck({ + enabled: true, + categories: { filler: true, redundancy: true, cliche: true }, + customPatterns: [], + onMatchesChange, + }), + ], + }); + const view = new EditorView({ state }); + + const matches = onMatchesChange.mock.lastCall?.[0] as StyleMatch[]; + const categories = new Set(matches.map((match) => match.category)); + const texts = matches.map((match) => match.text.toLowerCase()); + + expect(categories.has("filler")).toBeTruthy(); + expect(categories.has("redundancy")).toBeTruthy(); + expect(categories.has("cliche")).toBeTruthy(); + expect(texts).toContain("basically"); + expect(texts).toContain("in order to"); + expect(texts).toContain("beat around the bush"); + + view.destroy(); + }); + + it("applies configured marker style to style decorations", () => { + const state = EditorState.create({ + doc: "This is basically a test.", + extensions: [ + styleCheck({ + enabled: true, + categories: { filler: false, redundancy: false, cliche: false }, + customPatterns: [{ text: "basically", category: "filler" }], + markerStyle: "underline", + }), + ], + }); + const parent = document.createElement("div"); + document.body.append(parent); + const view = new EditorView({ state, parent }); + + const flagged = parent.querySelector(".style-flag"); + expect(flagged).toBeInTheDocument(); + expect(flagged).toHaveClass("style-marker-underline"); + expect(flagged).toHaveAttribute("data-marker-style", "underline"); + + view.destroy(); + parent.remove(); + }); +}); diff --git a/src/components/Editor.tsx b/src/components/Editor.tsx index f22b488..63ee740 100644 --- a/src/components/Editor.tsx +++ b/src/components/Editor.tsx @@ -36,6 +36,7 @@ export type EditorProps = { disabled?: boolean; placeholder?: string; debounceMs?: number; + styleSelection?: { from: number; to: number; requestId: number } | null; presentation?: EditorPresentationOverrides; onChange?: (text: string) => void; onSave?: () => void; @@ -135,6 +136,7 @@ function getStyleCheckExtension( category: p.category, replacement: p.replacement, })), + markerStyle: settings.markerStyle, onMatchesChange: onStyleMatchesChange, }), styleCheckTheme, @@ -210,6 +212,7 @@ export function Editor( disabled = false, placeholder, debounceMs = 500, + styleSelection = null, presentation, onChange, onSave, @@ -380,6 +383,7 @@ export function Editor( && previousPresentation.styleCheckSettings.categories.filler === styleCheckSettings.categories.filler && previousPresentation.styleCheckSettings.categories.redundancy === styleCheckSettings.categories.redundancy && previousPresentation.styleCheckSettings.categories.cliche === styleCheckSettings.categories.cliche + && previousPresentation.styleCheckSettings.markerStyle === styleCheckSettings.markerStyle && areCustomPatternsEqual( previousPresentation.styleCheckSettings.customPatterns, styleCheckSettings.customPatterns, @@ -444,6 +448,7 @@ export function Editor( || previousPresentation.styleCheckSettings.categories.filler !== styleCheckSettings.categories.filler || previousPresentation.styleCheckSettings.categories.redundancy !== styleCheckSettings.categories.redundancy || previousPresentation.styleCheckSettings.categories.cliche !== styleCheckSettings.categories.cliche + || previousPresentation.styleCheckSettings.markerStyle !== styleCheckSettings.markerStyle || !areCustomPatternsEqual( previousPresentation.styleCheckSettings.customPatterns, styleCheckSettings.customPatterns, @@ -485,6 +490,22 @@ export function Editor( view.dispatch({ changes: { from: 0, to: currentText.length, insert: initialText } }); }, [initialText]); + useEffect(() => { + const view = viewRef.current; + const styleSelectionFrom = styleSelection?.from ?? null; + const styleSelectionTo = styleSelection?.to ?? null; + if (!view || styleSelectionFrom === null || styleSelectionTo === null) { + return; + } + + const docLength = view.state.doc.length; + const from = Math.max(0, Math.min(docLength, styleSelectionFrom)); + const to = Math.max(from, Math.min(docLength, styleSelectionTo)); + + view.dispatch({ selection: { anchor: from, head: to }, scrollIntoView: true }); + view.focus(); + }, [styleSelection, styleSelection?.requestId]); + const focus = useCallback(() => { viewRef.current?.focus(); }, []); diff --git a/src/components/layout/AppHeaderBar.tsx b/src/components/layout/AppHeaderBar.tsx index e27d65f..1b34e86 100644 --- a/src/components/layout/AppHeaderBar.tsx +++ b/src/components/layout/AppHeaderBar.tsx @@ -1,7 +1,7 @@ import { Button } from "$components/Button"; import { useViewportTier } from "$hooks/useViewportTier"; -import { ChevronDownIcon, PenIcon, QuestionIcon, SearchIcon } from "$icons"; -import { useAppHeaderBarState, useHelpSheetState } from "$state/selectors"; +import { CheckIcon, ChevronDownIcon, PenIcon, QuestionIcon, SearchIcon } from "$icons"; +import { useAppHeaderBarState, useHelpSheetState, useStyleDiagnosticsUiState } from "$state/selectors"; import { useCallback, useMemo } from "react"; const AppTitle = ({ hideTitle }: { hideTitle: boolean }) => ( @@ -20,9 +20,11 @@ const SearchRow = ( onToggleSidebar, onToggleTabBar, onToggleStatusBar, + onToggleStyleDiagnostics, sidebarCollapsed, tabBarCollapsed, statusBarCollapsed, + styleDiagnosticsOpen, iconOnly, showSearchShortcut, showHelpShortcut, @@ -33,9 +35,11 @@ const SearchRow = ( onToggleSidebar: () => void; onToggleTabBar: () => void; onToggleStatusBar: () => void; + onToggleStyleDiagnostics: () => void; sidebarCollapsed: boolean; tabBarCollapsed: boolean; statusBarCollapsed: boolean; + styleDiagnosticsOpen: boolean; iconOnly: boolean; showSearchShortcut: boolean; showHelpShortcut: boolean; @@ -96,6 +100,16 @@ const SearchRow = ( Cmd+/ )} +