From 16a0011d5f6076a7cbdbf17324e213dae5cd4483 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Wed, 29 Apr 2026 13:03:27 +0200 Subject: [PATCH] refactor(sections): drop markdown section paste The H1-grouped markdown paste path created sections + tasks atomically from a hand-rolled parser, but the feature is niche, the parser is duplicated logic next to the existing flat-list and structured paste paths, and the documented atomicity caveat (partial state on concurrent sync) was never resolved. Falls back to the existing flat-list / sub-task paste paths, which cover the dominant use case. - Remove parseMarkdownWithSections + MarkdownWithSections / SectionWithTasks types. - Drop the section-paste branch and SectionService / WorkContextService deps from MarkdownPasteService. - Drop CONFIRM_SECTIONS i18n key. - Drop spec coverage for the removed parser. Net: -251 LOC. --- .../features/tasks/markdown-paste.service.ts | 99 ------------------- src/app/t.const.ts | 1 - src/app/util/parse-markdown-tasks.spec.ts | 60 ----------- src/app/util/parse-markdown-tasks.ts | 90 ----------------- src/assets/i18n/en.json | 1 - 5 files changed, 251 deletions(-) diff --git a/src/app/features/tasks/markdown-paste.service.ts b/src/app/features/tasks/markdown-paste.service.ts index 71fb0d47f2..580de6d7c3 100644 --- a/src/app/features/tasks/markdown-paste.service.ts +++ b/src/app/features/tasks/markdown-paste.service.ts @@ -5,7 +5,6 @@ import { parseMarkdownTasks, convertToMarkdownNotes, parseMarkdownTasksWithStructure, - parseMarkdownWithSections, } from '../../util/parse-markdown-tasks'; import { DialogConfirmComponent } from '../../ui/dialog-confirm/dialog-confirm.component'; import { T } from '../../t.const'; @@ -15,8 +14,6 @@ import { parseTimeSpentChanges } from './short-syntax'; import { GlobalConfigService } from '../config/global-config.service'; import { DEFAULT_GLOBAL_CONFIG } from '../config/default-global-config.const'; import { Task } from './task.model'; -import { SectionService } from '../section/section.service'; -import { WorkContextService } from '../work-context/work-context.service'; // Anchored at line start: ATX header (#…) or list-item marker (-/* with // optional checkbox). One-shot regex that bails before invoking the full @@ -31,8 +28,6 @@ export class MarkdownPasteService { private _taskService = inject(TaskService); private _store = inject(Store); private _globalConfigService = inject(GlobalConfigService); - private _sectionService = inject(SectionService); - private _workContextService = inject(WorkContextService); async handleMarkdownPaste( pastedText: string, @@ -77,95 +72,6 @@ export class MarkdownPasteService { return; } - // Try to parse with sections first (for markdown with H1 headers) - if (!selectedTaskId) { - const sectionsData = parseMarkdownWithSections(pastedText); - if (sectionsData) { - const totalTasks = sectionsData.sections.reduce( - (sum, section) => sum + section.tasks.length, - 0, - ); - const dialogRef = this._matDialog.open(DialogConfirmComponent, { - data: { - okTxt: T.G.CONFIRM, - title: T.F.MARKDOWN_PASTE.DIALOG_TITLE, - titleIcon: 'content_paste', - message: T.F.MARKDOWN_PASTE.CONFIRM_SECTIONS, - translateParams: { - sectionsCount: sectionsData.sections.length, - tasksCount: totalTasks, - }, - }, - }); - - const isConfirmed = await dialogRef.afterClosed().toPromise(); - if (!isConfirmed) { - return; - } - - const workContextId = this._workContextService.activeWorkContextId; - const sectionContextType = this._workContextService.activeWorkContextType; - if (!workContextId || !sectionContextType) { - return; - } - - // ATOMICITY NOTE — paste creates one section + N tasks + M sub-tasks. - // Each call below dispatches a separate action and produces a separate - // op-log entry. A sync push that lands mid-paste can leave a partial - // state on remote clients (e.g. a section with no tasks). The proper - // fix is a single bulk action reduced atomically across the - // task/section/project/tag reducers — tracked as follow-up. - // For now we do NOT yield to the event loop between dispatches: a - // mid-loop yield would widen the interleave window with concurrent - // sync replay (CLAUDE.md item 11 yields *after* a bulk apply, not - // inside one). The dispatches are synchronous so the whole paste - // completes in a single tick; a 100-task paste blocks for a few ms. - for (const section of sectionsData.sections) { - // Check post-trim so headers like `## ​` (zero-width space) or - // pure-whitespace section titles fall through to the noSection - // bucket instead of creating a titleless section. - const sectionId = section.sectionTitle?.trim() - ? this._sectionService.addSection( - section.sectionTitle, - workContextId, - sectionContextType, - ) - : null; - - for (const task of section.tasks) { - const taskId = this._taskService.add( - task.title, - false, - { - isDone: task.isCompleted, - notes: task.notes, - }, - true, - ); - - if (sectionId) { - this._sectionService.addTaskToSection(sectionId, taskId, null, null); - } - - if (task.subTasks && task.subTasks.length > 0) { - for (const subTask of task.subTasks) { - const subTaskObj = this._taskService.createNewTaskWithDefaults({ - title: subTask.title, - additional: { - isDone: subTask.isCompleted, - parentId: taskId, - notes: subTask.notes, - }, - }); - this._store.dispatch(addSubTask({ task: subTaskObj, parentId: taskId })); - } - } - } - } - return; - } - } - // Try to parse with structure first (for creating sub-tasks when no task selected) if (!selectedTaskId) { const structure = parseMarkdownTasksWithStructure(pastedText); @@ -290,11 +196,6 @@ export class MarkdownPasteService { // otherwise scan the whole input even for 800KB of prose. if (!MARKDOWN_TASK_OR_HEADER_RE.test(text)) return false; - // Sectioned (H1+) markdown — parseMarkdownWithSections returns null - // for header-less input. - if (parseMarkdownWithSections(text)) return true; - - // Flat task list fallback const parsedTasks = parseMarkdownTasks(text); return parsedTasks !== null && parsedTasks.length > 0; } diff --git a/src/app/t.const.ts b/src/app/t.const.ts index 4d289b04ba..5edd74f105 100644 --- a/src/app/t.const.ts +++ b/src/app/t.const.ts @@ -615,7 +615,6 @@ const T = { CONFIRM_PARENT_TASKS_WITH_SUBS: 'F.MARKDOWN_PASTE.CONFIRM_PARENT_TASKS_WITH_SUBS', CONFIRM_SUB_TASKS: 'F.MARKDOWN_PASTE.CONFIRM_SUB_TASKS', CONFIRM_SUB_TASKS_WITH_PARENT: 'F.MARKDOWN_PASTE.CONFIRM_SUB_TASKS_WITH_PARENT', - CONFIRM_SECTIONS: 'F.MARKDOWN_PASTE.CONFIRM_SECTIONS', DIALOG_TITLE: 'F.MARKDOWN_PASTE.DIALOG_TITLE', }, METRIC: { diff --git a/src/app/util/parse-markdown-tasks.spec.ts b/src/app/util/parse-markdown-tasks.spec.ts index d0a0363067..f6866eac68 100644 --- a/src/app/util/parse-markdown-tasks.spec.ts +++ b/src/app/util/parse-markdown-tasks.spec.ts @@ -1,7 +1,6 @@ import { parseMarkdownTasks, parseMarkdownTasksWithStructure, - parseMarkdownWithSections, } from './parse-markdown-tasks'; describe('parseMarkdownTasks', () => { @@ -365,62 +364,3 @@ describe('parseMarkdownTasksWithStructure', () => { }); }); }); - -describe('parseMarkdownWithSections', () => { - it('returns null when no header is present', () => { - expect(parseMarkdownWithSections('- a\n- b')).toBeNull(); - }); - - it('groups tasks under H1 headers', () => { - const result = parseMarkdownWithSections(`# One\n- a\n- b\n# Two\n- c`); - expect(result?.sections).toEqual([ - { - sectionTitle: 'One', - tasks: [ - { title: 'a', isCompleted: false }, - { title: 'b', isCompleted: false }, - ], - }, - { - sectionTitle: 'Two', - tasks: [{ title: 'c', isCompleted: false }], - }, - ]); - }); - - it('recognizes H2/H3 headers (not just H1)', () => { - const result = parseMarkdownWithSections(`## Two\n- a\n### Three\n- b`); - expect(result?.sections.map((s) => s.sectionTitle)).toEqual(['Two', 'Three']); - }); - - it('keeps tasks that appear before the first header as a "No Section" entry', () => { - const result = parseMarkdownWithSections(`- pre1\n- pre2\n# Header\n- post`); - expect(result?.sections[0]).toEqual({ - sectionTitle: null, - tasks: [ - { title: 'pre1', isCompleted: false }, - { title: 'pre2', isCompleted: false }, - ], - }); - expect(result?.sections[1]).toEqual({ - sectionTitle: 'Header', - tasks: [{ title: 'post', isCompleted: false }], - }); - }); - - it('produces an empty tasks array for headers with no following tasks', () => { - const result = parseMarkdownWithSections(`# Empty\n# Has Tasks\n- a`); - expect(result?.sections).toEqual([ - { sectionTitle: 'Empty', tasks: [] }, - { - sectionTitle: 'Has Tasks', - tasks: [{ title: 'a', isCompleted: false }], - }, - ]); - }); - - it('returns null for null/empty input', () => { - expect(parseMarkdownWithSections('')).toBeNull(); - expect(parseMarkdownWithSections(null as unknown as string)).toBeNull(); - }); -}); diff --git a/src/app/util/parse-markdown-tasks.ts b/src/app/util/parse-markdown-tasks.ts index 3871d66089..358baa95eb 100644 --- a/src/app/util/parse-markdown-tasks.ts +++ b/src/app/util/parse-markdown-tasks.ts @@ -36,15 +36,6 @@ export interface MarkdownTaskStructure { totalSubTasks: number; } -export interface SectionWithTasks { - sectionTitle: string | null; // null for "No Section" - tasks: ParsedMarkdownTask[]; -} - -export interface MarkdownWithSections { - sections: SectionWithTasks[]; -} - interface ParsedLine { indentLevel: number; content: string; @@ -341,84 +332,3 @@ export const parseMarkdownTasks = (text: string): ParsedMarkdownTask[] | null => // Return tasks only if we found at least one return tasks.length > 0 ? tasks : null; }; - -/** - * Parse markdown text to detect ATX headers (`#` to `######`) and group - * tasks under sections. Returns `null` if no headers are present so callers - * can fall through to a flat-task parse. - * - * FOLLOW-UP: this hand-rolled parser only treats `-`/`*` bullets as task - * lines and silently drops `1.`-style ordered lists. The repo already - * depends on `marked` (see `marked-options-factory.ts`); rewriting on - * top of `marked.lexer(text)` would shrink this and the two near-clones - * (`parseMarkdownTasks`, `parseMarkdownTasksWithStructure`) and pick up - * GFM task-list semantics for free. Out of scope for this PR. - * - * @param text - Markdown text with potential headers and task lists - * @returns Sections with tasks, or null if not valid markdown - */ -export const parseMarkdownWithSections = (text: string): MarkdownWithSections | null => { - if (!text || typeof text !== 'string') { - return null; - } - if (text.length > MAX_INPUT_LENGTH) return null; - - // Mirror splitMarkdownLines but preserve empty lines — section parsing - // uses them as separators. - const lines = text.replace(/^/, '').replace(/\r\n?/g, '\n').split('\n'); - const sections: SectionWithTasks[] = []; - let currentSection: SectionWithTasks | null = null; - let hasHeaders = false; - const pendingTaskLines: string[] = []; - - // Flush pendingTaskLines into a section. If currentSection is null (i.e. we - // are about to enter the first header but already collected tasks above it), - // create a "No Section" entry at the top so those tasks aren't dropped. - const flushPending = (): void => { - if (pendingTaskLines.length === 0) return; - const parsedTasks = parseMarkdownTasksWithStructure(pendingTaskLines.join('\n')); - pendingTaskLines.length = 0; - if (!parsedTasks) return; - if (currentSection) { - currentSection.tasks = parsedTasks.mainTasks; - } else { - sections.unshift({ sectionTitle: null, tasks: parsedTasks.mainTasks }); - } - }; - - const headerRegex = /^#{1,6}\s+(.+)$/; - - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - const trimmed = line.trim(); - - // Detect ATX header (#, ##, ### up to ######) - const headerMatch = trimmed.match(headerRegex); - if (headerMatch) { - hasHeaders = true; - flushPending(); - - currentSection = { - sectionTitle: headerMatch[1].trim(), - tasks: [], - }; - sections.push(currentSection); - } - // Detect task lines - else if (trimmed.match(/^[-*]\s+/) || trimmed.match(/^[-*]\s*\[([ x])\]/)) { - pendingTaskLines.push(line); - } - // Empty lines and other content are ignored for now - } - - flushPending(); - - // Only return when at least one header was seen — callers treat - // header-less input as a flat task list and route through - // `parseMarkdownTasks`. - if (!hasHeaders) { - return null; - } - - return { sections }; -}; diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index 1982d3cc51..508c880625 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -607,7 +607,6 @@ "CONFIRM_ADD_TO_SUB_TASK_NOTES": "Add the pasted Markdown list to the notes of subtask \"{{parentTaskTitle}}\"?", "CONFIRM_PARENT_TASKS": "Create {{tasksCount}} new tasks from the pasted Markdown list?", "CONFIRM_PARENT_TASKS_WITH_SUBS": "Create {{tasksCount}} new tasks and {{subTasksCount}} subtasks from the pasted Markdown list?", - "CONFIRM_SECTIONS": "Create {{sectionsCount}} section(s) with {{tasksCount}} task(s)?", "CONFIRM_SUB_TASKS": "Create {{tasksCount}} new subtasks from the pasted Markdown list?", "CONFIRM_SUB_TASKS_WITH_PARENT": "Create {{tasksCount}} new subtasks under \"{{parentTaskTitle}}\" from the pasted Markdown list?", "DIALOG_TITLE": "Pasted Markdown list detected!"