Skip to content

Commit 690c9c0

Browse files
couimetcouimet
authored
[issues/298] Warn user when generating link from file with unsaved changes (#305)
* [issues/298] Add message codes for dirty buffer warning Adds i18n message codes and English translations for the dirty buffer warning feature that will alert users when generating links from files with unsaved changes. Message codes added: - WARN_LINK_DIRTY_BUFFER: Warning message text - WARN_LINK_DIRTY_BUFFER_SAVE: "Save & Generate" button label - WARN_LINK_DIRTY_BUFFER_CONTINUE: "Generate Anyway" button label * [issues/298] Add warnOnDirtyBuffer configuration setting Adds the `rangelink.warnOnDirtyBuffer` setting (default: true) that controls whether a warning is shown when generating a link from a file with unsaved changes. Changes: - Added setting to package.json contributes.configuration - Added SETTING_WARN_ON_DIRTY_BUFFER constant - Added ConfigReader.getBoolean() method for boolean settings - Added tests for the new config method and package.json contract * [issues/298] Implement dirty buffer warning in `generateLinkFromSelection()` Adds a warning dialog when generating a link from a file with unsaved changes. This helps users avoid creating links that may point to incorrect positions after the file is saved. User options: - "Save & Generate": Save the file first, then generate the link - "Generate Anyway": Generate the link without saving - Dismiss: Abort link generation The warning respects the `rangelink.warnOnDirtyBuffer` setting (default: true). Config is only read when document is actually dirty (optimization). Implementation uses `DirtyBufferWarningResult` enum for richer API instead of boolean return values. Uses switch statement with exhaustive checking and UNEXPECTED_CODE_PATH default. Changes: - DirtyBufferWarningResult.ts: New enum for warning dialog outcomes - RangeLinkService.ts: Added handleDirtyBufferWarning() with switch/exhaustive check pattern - RangeLinkService.test.ts: Added 6 tests for dirty buffer warning behavior - createMockConfigReader.ts: Added getBoolean support - CHANGELOG.md: Added entry for dirty buffer warning feature - README.md: Documented warnOnDirtyBuffer setting with restructured Configuration section * Ran `fix:format` * [PR feedback] Fix import order and handle `document.save()` failure Addresses CodeRabbit review feedback on PR #305: 1. Import order: Move `import type` statements before value imports to satisfy ESLint import/order rule. 2. Save result handling: `document.save()` returns a boolean — if save fails or is cancelled, link generation now aborts with a user-facing warning instead of silently proceeding with a dirty buffer. Adds `SaveFailed` enum value to `DirtyBufferWarningResult` for semantic clarity (distinct from `Dismissed`). Ref: #305 (review)
1 parent d8de171 commit 690c9c0

File tree

14 files changed

+390
-13
lines changed

14 files changed

+390
-13
lines changed

‎packages/rangelink-vscode-extension/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- **Dirty Buffer Warning** - Warning when generating a link from a file with unsaved changes (#298)
13+
- Shows a dialog with options: "Save & Generate", "Generate Anyway", or dismiss to abort
14+
- Controlled by `rangelink.warnOnDirtyBuffer` setting (default: `true`)
15+
- Helps avoid creating links that may point to incorrect positions after the file is saved
1216
- **Paste File Path Commands** - Send file paths directly to bound destinations (#243)
1317
- **Context menu commands** - See "Context Menu Integrations" section below for full details
1418
- **Command palette commands** - For the currently active editor

‎packages/rangelink-vscode-extension/README.md‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -319,16 +319,16 @@ Positioned after VSCode's "Copy Path" / "Copy Relative Path":
319319

320320
## Configuration
321321

322-
Customize delimiters in VSCode settings (Preferences > Settings > search "rangelink"):
323-
324-
```json
325-
{
326-
"rangelink.delimiterLine": "L",
327-
"rangelink.delimiterPosition": "C",
328-
"rangelink.delimiterHash": "#",
329-
"rangelink.delimiterRange": "-"
330-
}
331-
```
322+
Customize settings in VSCode (Preferences > Settings > search "rangelink").
323+
324+
### Delimiter Settings
325+
326+
| Setting | Default | Description |
327+
| ----------------------------- | ------- | ----------------------------------- |
328+
| `rangelink.delimiterLine` | `"L"` | Line number prefix |
329+
| `rangelink.delimiterPosition` | `"C"` | Column/character position prefix |
330+
| `rangelink.delimiterHash` | `"#"` | Separator between path and location |
331+
| `rangelink.delimiterRange` | `"-"` | Range separator (start-end) |
332332

333333
**Validation Rules:**
334334

@@ -339,6 +339,14 @@ Customize delimiters in VSCode settings (Preferences > Settings > search "rangel
339339

340340
Invalid configurations will fall back to defaults with a warning in the output channel (`Cmd+Shift+U` / `Ctrl+Shift+U`, select "RangeLink"). See [DEVELOPMENT.md](./DEVELOPMENT.md#development-workflow) for details.
341341

342+
### Warning Settings
343+
344+
| Setting | Default | Description |
345+
| ----------------------------- | ------- | -------------------------------------------------------------------- |
346+
| `rangelink.warnOnDirtyBuffer` | `true` | Show warning when generating a link from a file with unsaved changes |
347+
348+
When enabled, a dialog appears with options: "Save & Generate", "Generate Anyway", or dismiss to abort. This helps avoid creating links that may point to incorrect positions after the file is saved.
349+
342350
## What's Next
343351

344352
RangeLink is under active development. See [open issues](https://github.com/couimet/rangeLink/issues) for planned features and enhancement requests.

‎packages/rangelink-vscode-extension/package.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,11 @@
387387
"No padding (paste bookmark as-is)"
388388
],
389389
"description": "Smart padding for saved bookmarks when pasting to destinations"
390+
},
391+
"rangelink.warnOnDirtyBuffer": {
392+
"type": "boolean",
393+
"default": true,
394+
"description": "Show warning when generating link from file with unsaved changes"
390395
}
391396
}
392397
},

‎packages/rangelink-vscode-extension/src/RangeLinkService.ts‎

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,20 @@ import {
1010
SETTING_SMART_PADDING_PASTE_CONTENT,
1111
SETTING_SMART_PADDING_PASTE_FILE_PATH,
1212
SETTING_SMART_PADDING_PASTE_LINK,
13+
SETTING_WARN_ON_DIRTY_BUFFER,
1314
} from './constants';
1415
import type { PasteDestination } from './destinations/PasteDestination';
1516
import type { PasteDestinationManager } from './destinations/PasteDestinationManager';
17+
import { RangeLinkExtensionError } from './errors/RangeLinkExtensionError';
18+
import { RangeLinkExtensionErrorCodes } from './errors/RangeLinkExtensionErrorCodes';
1619
import { VscodeAdapter } from './ide/vscode/VscodeAdapter';
17-
import { ActiveSelections, MessageCode, PasteContentType, QuickPickBindResult } from './types';
20+
import {
21+
ActiveSelections,
22+
DirtyBufferWarningResult,
23+
MessageCode,
24+
PasteContentType,
25+
QuickPickBindResult,
26+
} from './types';
1827
import {
1928
formatMessage,
2029
generateLinkFromSelections,
@@ -353,6 +362,25 @@ export class RangeLinkService {
353362

354363
const { editor, selections } = validated;
355364
const document = editor.document;
365+
366+
if (document.isDirty) {
367+
const shouldWarnOnDirty = this.configReader.getBoolean(SETTING_WARN_ON_DIRTY_BUFFER, true);
368+
if (shouldWarnOnDirty) {
369+
const warningResult = await this.handleDirtyBufferWarning(document);
370+
if (
371+
warningResult === DirtyBufferWarningResult.Dismissed ||
372+
warningResult === DirtyBufferWarningResult.SaveFailed
373+
) {
374+
return undefined;
375+
}
376+
} else {
377+
this.logger.debug(
378+
{ fn: 'generateLinkFromSelection', documentUri: document.uri.toString() },
379+
'Document has unsaved changes but warning is disabled by setting',
380+
);
381+
}
382+
}
383+
356384
const referencePath = this.getReferencePath(document.uri, pathFormat);
357385
const linkType = isPortable ? LinkType.Portable : LinkType.Regular;
358386

@@ -386,6 +414,72 @@ export class RangeLinkService {
386414
return formattedLink;
387415
}
388416

417+
/**
418+
* Handles the dirty buffer warning dialog.
419+
*
420+
* @param document The document with unsaved changes
421+
* @returns The user's choice from the warning dialog
422+
*/
423+
private async handleDirtyBufferWarning(
424+
document: vscode.TextDocument,
425+
): Promise<DirtyBufferWarningResult> {
426+
const warningMessage = formatMessage(MessageCode.WARN_LINK_DIRTY_BUFFER);
427+
const saveAndGenerateLabel = formatMessage(MessageCode.WARN_LINK_DIRTY_BUFFER_SAVE);
428+
const generateAnywayLabel = formatMessage(MessageCode.WARN_LINK_DIRTY_BUFFER_CONTINUE);
429+
430+
this.logger.debug(
431+
{ fn: 'handleDirtyBufferWarning', documentUri: document.uri.toString() },
432+
'Document has unsaved changes, showing warning',
433+
);
434+
435+
const choice = await this.ideAdapter.showWarningMessage(
436+
warningMessage,
437+
saveAndGenerateLabel,
438+
generateAnywayLabel,
439+
);
440+
441+
const result: DirtyBufferWarningResult =
442+
choice === saveAndGenerateLabel
443+
? DirtyBufferWarningResult.SaveAndGenerate
444+
: choice === generateAnywayLabel
445+
? DirtyBufferWarningResult.GenerateAnyway
446+
: DirtyBufferWarningResult.Dismissed;
447+
448+
switch (result) {
449+
case DirtyBufferWarningResult.SaveAndGenerate: {
450+
this.logger.debug({ fn: 'handleDirtyBufferWarning' }, 'User chose to save and generate');
451+
const saved = await document.save();
452+
if (!saved) {
453+
this.logger.warn(
454+
{ fn: 'handleDirtyBufferWarning' },
455+
'Save operation failed or was cancelled',
456+
);
457+
this.ideAdapter.showWarningMessage(
458+
formatMessage(MessageCode.WARN_LINK_DIRTY_BUFFER_SAVE_FAILED),
459+
);
460+
return DirtyBufferWarningResult.SaveFailed;
461+
}
462+
this.logger.debug({ fn: 'handleDirtyBufferWarning' }, 'Document saved successfully');
463+
return result;
464+
}
465+
case DirtyBufferWarningResult.GenerateAnyway:
466+
this.logger.debug({ fn: 'handleDirtyBufferWarning' }, 'User chose to generate anyway');
467+
return result;
468+
case DirtyBufferWarningResult.Dismissed:
469+
this.logger.debug({ fn: 'handleDirtyBufferWarning' }, 'User dismissed warning, aborting');
470+
return result;
471+
default: {
472+
const exhaustiveCheck: never = result;
473+
throw new RangeLinkExtensionError({
474+
code: RangeLinkExtensionErrorCodes.UNEXPECTED_CODE_PATH,
475+
message: `Unexpected dirty buffer warning result: ${exhaustiveCheck}`,
476+
functionName: 'handleDirtyBufferWarning',
477+
details: { exhaustiveCheck },
478+
});
479+
}
480+
}
481+
}
482+
389483
/**
390484
* Copies the link to clipboard and sends to bound destination if available
391485
*

‎packages/rangelink-vscode-extension/src/__tests__/RangeLinkService.test.ts‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1725,6 +1725,183 @@ describe('RangeLinkService', () => {
17251725
});
17261726
});
17271727

1728+
describe('generateLinkFromSelection() - dirty buffer warning', () => {
1729+
let mockGenerateLinkFromSelections: jest.SpyInstance;
1730+
let mockShowWarningMessage: jest.Mock;
1731+
let mockDocumentSave: jest.Mock;
1732+
1733+
beforeEach(() => {
1734+
mockShowWarningMessage = jest.fn();
1735+
mockDocumentSave = jest.fn().mockResolvedValue(true);
1736+
1737+
mockGenerateLinkFromSelections = jest.spyOn(generateLinkModule, 'generateLinkFromSelections');
1738+
mockGenerateLinkFromSelections.mockReturnValue(
1739+
Result.ok(createMockFormattedLink('file.ts#L10')),
1740+
);
1741+
});
1742+
1743+
const createServiceWithDirtyDocument = (isDirty: boolean, warnOnDirtyBuffer: boolean) => {
1744+
const mockDocument = createMockDocument({
1745+
uri: createMockUri('/test/file.ts'),
1746+
isDirty,
1747+
save: mockDocumentSave,
1748+
});
1749+
const mockEditor = createMockEditor({
1750+
document: mockDocument,
1751+
selections: [createMockSelection({ isEmpty: false })],
1752+
});
1753+
1754+
const localMockVscodeAdapter = createMockVscodeAdapter({
1755+
windowOptions: {
1756+
activeTextEditor: mockEditor,
1757+
showWarningMessage: mockShowWarningMessage,
1758+
},
1759+
workspaceOptions: {
1760+
getWorkspaceFolder: jest
1761+
.fn()
1762+
.mockReturnValue({ uri: createMockUri('/test'), index: 0, name: 'test' }),
1763+
asRelativePath: jest.fn().mockReturnValue('file.ts'),
1764+
},
1765+
});
1766+
1767+
const localMockConfigReader = createMockConfigReader({
1768+
getBoolean: jest.fn((key: string, defaultValue: boolean) => {
1769+
if (key === 'warnOnDirtyBuffer') return warnOnDirtyBuffer;
1770+
return defaultValue;
1771+
}),
1772+
});
1773+
1774+
const localService = new RangeLinkService(
1775+
delimiters,
1776+
localMockVscodeAdapter,
1777+
createMockDestinationManager({ isBound: false }),
1778+
localMockConfigReader,
1779+
mockLogger,
1780+
);
1781+
1782+
return { service: localService, mockDocument, mockVscodeAdapter: localMockVscodeAdapter };
1783+
};
1784+
1785+
it('should show warning when document is dirty and setting is enabled', async () => {
1786+
const { service: localService } = createServiceWithDirtyDocument(true, true);
1787+
mockShowWarningMessage.mockResolvedValue('Generate Anyway');
1788+
1789+
await (localService as any).generateLinkFromSelection(PathFormat.WorkspaceRelative, false);
1790+
1791+
expect(mockShowWarningMessage).toHaveBeenCalledWith(
1792+
'File has unsaved changes. Link may point to wrong position after save.',
1793+
'Save & Generate',
1794+
'Generate Anyway',
1795+
);
1796+
expect(mockLogger.debug).toHaveBeenCalledWith(
1797+
{ fn: 'handleDirtyBufferWarning', documentUri: 'file:///test/file.ts' },
1798+
'Document has unsaved changes, showing warning',
1799+
);
1800+
expect(mockLogger.debug).toHaveBeenCalledWith(
1801+
{ fn: 'handleDirtyBufferWarning' },
1802+
'User chose to generate anyway',
1803+
);
1804+
});
1805+
1806+
it('should NOT show warning when document is dirty but setting is disabled', async () => {
1807+
const { service: localService } = createServiceWithDirtyDocument(true, false);
1808+
1809+
await (localService as any).generateLinkFromSelection(PathFormat.WorkspaceRelative, false);
1810+
1811+
expect(mockShowWarningMessage).not.toHaveBeenCalled();
1812+
expect(mockGenerateLinkFromSelections).toHaveBeenCalled();
1813+
expect(mockLogger.debug).toHaveBeenCalledWith(
1814+
{ fn: 'generateLinkFromSelection', documentUri: 'file:///test/file.ts' },
1815+
'Document has unsaved changes but warning is disabled by setting',
1816+
);
1817+
});
1818+
1819+
it('should NOT show warning when document is not dirty', async () => {
1820+
const { service: localService } = createServiceWithDirtyDocument(false, true);
1821+
1822+
await (localService as any).generateLinkFromSelection(PathFormat.WorkspaceRelative, false);
1823+
1824+
expect(mockShowWarningMessage).not.toHaveBeenCalled();
1825+
expect(mockGenerateLinkFromSelections).toHaveBeenCalled();
1826+
});
1827+
1828+
it('should save document and continue when user chooses "Save & Generate"', async () => {
1829+
const { service: localService, mockDocument } = createServiceWithDirtyDocument(true, true);
1830+
mockShowWarningMessage.mockResolvedValue('Save & Generate');
1831+
1832+
const result = await (localService as any).generateLinkFromSelection(
1833+
PathFormat.WorkspaceRelative,
1834+
false,
1835+
);
1836+
1837+
expect(mockDocument.save).toHaveBeenCalled();
1838+
expect(mockGenerateLinkFromSelections).toHaveBeenCalled();
1839+
expect(result).toBeDefined();
1840+
expect(mockLogger.debug).toHaveBeenCalledWith(
1841+
{ fn: 'handleDirtyBufferWarning' },
1842+
'User chose to save and generate',
1843+
);
1844+
});
1845+
1846+
it('should continue without saving when user chooses "Generate Anyway"', async () => {
1847+
const { service: localService, mockDocument } = createServiceWithDirtyDocument(true, true);
1848+
mockShowWarningMessage.mockResolvedValue('Generate Anyway');
1849+
1850+
const result = await (localService as any).generateLinkFromSelection(
1851+
PathFormat.WorkspaceRelative,
1852+
false,
1853+
);
1854+
1855+
expect(mockDocument.save).not.toHaveBeenCalled();
1856+
expect(mockGenerateLinkFromSelections).toHaveBeenCalled();
1857+
expect(result).toBeDefined();
1858+
expect(mockLogger.debug).toHaveBeenCalledWith(
1859+
{ fn: 'handleDirtyBufferWarning' },
1860+
'User chose to generate anyway',
1861+
);
1862+
});
1863+
1864+
it('should abort generation when user dismisses warning', async () => {
1865+
const { service: localService, mockDocument } = createServiceWithDirtyDocument(true, true);
1866+
mockShowWarningMessage.mockResolvedValue(undefined);
1867+
1868+
const result = await (localService as any).generateLinkFromSelection(
1869+
PathFormat.WorkspaceRelative,
1870+
false,
1871+
);
1872+
1873+
expect(mockDocument.save).not.toHaveBeenCalled();
1874+
expect(mockGenerateLinkFromSelections).not.toHaveBeenCalled();
1875+
expect(result).toBeUndefined();
1876+
expect(mockLogger.debug).toHaveBeenCalledWith(
1877+
{ fn: 'handleDirtyBufferWarning' },
1878+
'User dismissed warning, aborting',
1879+
);
1880+
});
1881+
1882+
it('should abort generation and show warning when save fails', async () => {
1883+
mockDocumentSave.mockResolvedValue(false);
1884+
const { service: localService, mockDocument } = createServiceWithDirtyDocument(true, true);
1885+
mockShowWarningMessage.mockResolvedValue('Save & Generate');
1886+
1887+
const result = await (localService as any).generateLinkFromSelection(
1888+
PathFormat.WorkspaceRelative,
1889+
false,
1890+
);
1891+
1892+
expect(mockDocument.save).toHaveBeenCalled();
1893+
expect(mockGenerateLinkFromSelections).not.toHaveBeenCalled();
1894+
expect(result).toBeUndefined();
1895+
expect(mockLogger.warn).toHaveBeenCalledWith(
1896+
{ fn: 'handleDirtyBufferWarning' },
1897+
'Save operation failed or was cancelled',
1898+
);
1899+
expect(mockShowWarningMessage).toHaveBeenCalledWith(
1900+
'File could not be saved. Link generation aborted.',
1901+
);
1902+
});
1903+
});
1904+
17281905
describe('generateLinkFromSelection() - path resolution', () => {
17291906
let mockGenerateLinkFromSelections: jest.SpyInstance;
17301907

‎packages/rangelink-vscode-extension/src/__tests__/constants/packageJsonContracts.test.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ interface CommandContribution {
1010

1111
interface ConfigurationProperty {
1212
type: string;
13-
default: string;
13+
default: string | boolean;
1414
description: string;
1515
pattern?: string;
1616
enum?: string[];
@@ -534,8 +534,18 @@ describe('package.json contributions', () => {
534534
});
535535
});
536536

537+
describe('warning settings', () => {
538+
it('rangelink.warnOnDirtyBuffer', () => {
539+
expect(properties['rangelink.warnOnDirtyBuffer']).toStrictEqual({
540+
type: 'boolean',
541+
default: true,
542+
description: 'Show warning when generating link from file with unsaved changes',
543+
});
544+
});
545+
});
546+
537547
it('has the expected number of configuration properties', () => {
538-
expect(Object.keys(properties)).toHaveLength(8);
548+
expect(Object.keys(properties)).toHaveLength(9);
539549
});
540550
});
541551

0 commit comments

Comments
 (0)