Compare commits
2 Commits
efbe6fe77f
...
b1701865ab
| Author | SHA1 | Date | |
|---|---|---|---|
| b1701865ab | |||
| edb29a9376 |
@@ -0,0 +1,87 @@
|
||||
# Design: Line Numbers by Default in `read_note`
|
||||
|
||||
**Date:** 2026-01-31
|
||||
**Version:** 1.2.1
|
||||
|
||||
## Summary
|
||||
|
||||
Change `read_note` to return line-numbered content by default (e.g., `1→First line`) to help AI assistants reference specific locations when discussing notes. Add `withLineNumbers: false` to get raw content.
|
||||
|
||||
## Motivation
|
||||
|
||||
AI assistants can give much more precise references like "line 42 has a typo" rather than vague "in the section about X". Line numbers make file discussions unambiguous.
|
||||
|
||||
## Changes
|
||||
|
||||
### 1. Default Behavior Change
|
||||
|
||||
```typescript
|
||||
// Before
|
||||
const withLineNumbers = options?.withLineNumbers ?? false;
|
||||
|
||||
// After
|
||||
const withLineNumbers = options?.withLineNumbers ?? true;
|
||||
```
|
||||
|
||||
### 2. Apply to Parsed Path
|
||||
|
||||
Currently the `parseFrontmatter: true` path ignores line numbers. Add line numbering to the `content` field (and `contentWithoutFrontmatter`) when enabled.
|
||||
|
||||
### 3. Schema Update
|
||||
|
||||
Update the tool description to say "Default: true" and clarify opt-out with `withLineNumbers: false`.
|
||||
|
||||
## Files to Modify
|
||||
|
||||
### `src/tools/note-tools.ts`
|
||||
- Line 48: Change default from `false` to `true`
|
||||
- Lines 125-155: Add line numbering logic to the `parseFrontmatter` path for `content` and `contentWithoutFrontmatter` fields
|
||||
- Add `totalLines` to parsed response when line numbers enabled
|
||||
|
||||
### `src/tools/index.ts`
|
||||
- Lines 51-54: Update schema description to reflect new default
|
||||
|
||||
### `tests/note-tools.test.ts`
|
||||
- Update existing tests that expect raw content to either:
|
||||
- Explicitly pass `withLineNumbers: false`, or
|
||||
- Update assertions to expect numbered content
|
||||
|
||||
## Response Format Examples
|
||||
|
||||
### Before (current default)
|
||||
```json
|
||||
{
|
||||
"content": "# Title\nSome content",
|
||||
"wordCount": 3,
|
||||
"versionId": "abc123"
|
||||
}
|
||||
```
|
||||
|
||||
### After (new default)
|
||||
```json
|
||||
{
|
||||
"content": "1→# Title\n2→Some content",
|
||||
"totalLines": 2,
|
||||
"wordCount": 3,
|
||||
"versionId": "abc123"
|
||||
}
|
||||
```
|
||||
|
||||
### Opt-out (`withLineNumbers: false`)
|
||||
```json
|
||||
{
|
||||
"content": "# Title\nSome content",
|
||||
"wordCount": 3,
|
||||
"versionId": "abc123"
|
||||
}
|
||||
```
|
||||
|
||||
## Breaking Change
|
||||
|
||||
This changes the default response format. MCP clients that parse `content` expecting raw text will need to either:
|
||||
- Update their parsing to handle line-numbered format
|
||||
- Explicitly pass `withLineNumbers: false`
|
||||
|
||||
## Version
|
||||
|
||||
Bump to 1.2.1.
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"id": "mcp-server",
|
||||
"name": "MCP Server",
|
||||
"version": "1.2.0",
|
||||
"version": "1.2.1",
|
||||
"minAppVersion": "0.15.0",
|
||||
"description": "Exposes vault operations via Model Context Protocol (MCP) over HTTP.",
|
||||
"author": "William Ballou",
|
||||
|
||||
4
package-lock.json
generated
4
package-lock.json
generated
@@ -1,12 +1,12 @@
|
||||
{
|
||||
"name": "mcp-server",
|
||||
"version": "1.1.4",
|
||||
"version": "1.2.1",
|
||||
"lockfileVersion": 3,
|
||||
"requires": true,
|
||||
"packages": {
|
||||
"": {
|
||||
"name": "mcp-server",
|
||||
"version": "1.1.4",
|
||||
"version": "1.2.1",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"cors": "^2.8.5",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mcp-server",
|
||||
"version": "1.2.0",
|
||||
"version": "1.2.1",
|
||||
"description": "MCP (Model Context Protocol) server plugin - exposes vault operations via HTTP",
|
||||
"main": "main.js",
|
||||
"scripts": {
|
||||
|
||||
@@ -50,7 +50,7 @@ export class ToolRegistry {
|
||||
},
|
||||
withLineNumbers: {
|
||||
type: "boolean",
|
||||
description: "If true, prefix each line with its line number (e.g., '1→content'). Use this when you need to make line-based edits with update_sections. Returns totalLines count and versionId for use with ifMatch parameter. Default: false"
|
||||
description: "If true (default), prefix each line with its line number (e.g., '1→content'). This helps AI assistants reference specific line numbers when discussing notes. Returns totalLines count and versionId for use with ifMatch parameter. Set to false to get raw content without line prefixes. Default: true"
|
||||
}
|
||||
},
|
||||
required: ["path"]
|
||||
|
||||
@@ -45,7 +45,7 @@ export class NoteTools {
|
||||
/* istanbul ignore next */
|
||||
const parseFrontmatter = options?.parseFrontmatter ?? false;
|
||||
/* istanbul ignore next */
|
||||
const withLineNumbers = options?.withLineNumbers ?? false;
|
||||
const withLineNumbers = options?.withLineNumbers ?? true;
|
||||
|
||||
// Validate path
|
||||
if (!path || path.trim() === '') {
|
||||
@@ -125,13 +125,38 @@ export class NoteTools {
|
||||
// Parse frontmatter if requested
|
||||
const extracted = FrontmatterUtils.extractFrontmatter(content);
|
||||
|
||||
// Apply line numbers if requested
|
||||
let resultContent = withContent ? content : '';
|
||||
let resultContentWithoutFrontmatter = extracted.contentWithoutFrontmatter;
|
||||
let totalLines: number | undefined;
|
||||
|
||||
if (withLineNumbers && withContent) {
|
||||
const lines = content.split('\n');
|
||||
resultContent = lines.map((line, idx) => `${idx + 1}→${line}`).join('\n');
|
||||
totalLines = lines.length;
|
||||
|
||||
if (extracted.hasFrontmatter && extracted.contentWithoutFrontmatter) {
|
||||
const contentLines = extracted.contentWithoutFrontmatter.split('\n');
|
||||
// Calculate the offset: frontmatter lines + 1 for the empty line after ---
|
||||
const frontmatterLineCount = extracted.frontmatter ? extracted.frontmatter.split('\n').length + 2 : 0;
|
||||
resultContentWithoutFrontmatter = contentLines
|
||||
.map((line, idx) => `${frontmatterLineCount + idx + 1}→${line}`)
|
||||
.join('\n');
|
||||
}
|
||||
}
|
||||
|
||||
const result: ParsedNote = {
|
||||
path: file.path,
|
||||
hasFrontmatter: extracted.hasFrontmatter,
|
||||
/* istanbul ignore next - Conditional content inclusion tested via integration tests */
|
||||
content: withContent ? content : ''
|
||||
content: resultContent
|
||||
};
|
||||
|
||||
// Add totalLines when line numbers are enabled
|
||||
if (totalLines !== undefined) {
|
||||
result.totalLines = totalLines;
|
||||
}
|
||||
|
||||
// Include frontmatter if requested
|
||||
/* istanbul ignore next - Response building branches tested via integration tests */
|
||||
if (withFrontmatter && extracted.hasFrontmatter) {
|
||||
@@ -142,7 +167,7 @@ export class NoteTools {
|
||||
// Include content without frontmatter if parsing
|
||||
/* istanbul ignore next */
|
||||
if (withContent && extracted.hasFrontmatter) {
|
||||
result.contentWithoutFrontmatter = extracted.contentWithoutFrontmatter;
|
||||
result.contentWithoutFrontmatter = resultContentWithoutFrontmatter;
|
||||
}
|
||||
|
||||
// Add word count when content is included
|
||||
|
||||
@@ -218,6 +218,7 @@ export interface ParsedNote {
|
||||
content: string;
|
||||
contentWithoutFrontmatter?: string;
|
||||
wordCount?: number;
|
||||
totalLines?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -52,7 +52,7 @@ describe('NoteTools', () => {
|
||||
});
|
||||
|
||||
describe('readNote', () => {
|
||||
it('should read note content successfully', async () => {
|
||||
it('should read note content successfully with line numbers by default', async () => {
|
||||
const mockFile = createMockTFile('test.md');
|
||||
const content = '# Test Note\n\nThis is test content.';
|
||||
|
||||
@@ -62,9 +62,10 @@ describe('NoteTools', () => {
|
||||
const result = await noteTools.readNote('test.md');
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
// Now returns JSON with content and wordCount
|
||||
// Now returns JSON with content (line-numbered by default) and wordCount
|
||||
const parsed = JSON.parse(result.content[0].text);
|
||||
expect(parsed.content).toBe(content);
|
||||
expect(parsed.content).toBe('1→# Test Note\n2→\n3→This is test content.');
|
||||
expect(parsed.totalLines).toBe(3);
|
||||
expect(parsed.wordCount).toBe(7); // Test Note This is test content
|
||||
expect(mockVault.read).toHaveBeenCalledWith(mockFile);
|
||||
});
|
||||
@@ -124,7 +125,7 @@ describe('NoteTools', () => {
|
||||
(PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile);
|
||||
mockVault.read = jest.fn().mockResolvedValue(content);
|
||||
|
||||
const result = await noteTools.readNote('test.md', { withContent: true });
|
||||
const result = await noteTools.readNote('test.md', { withContent: true, withLineNumbers: false });
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
const parsed = JSON.parse(result.content[0].text);
|
||||
@@ -188,23 +189,23 @@ describe('NoteTools', () => {
|
||||
expect(parsed.wordCount).toBe(0);
|
||||
});
|
||||
|
||||
it('should return JSON format even with default options', async () => {
|
||||
it('should return JSON format with raw content when withLineNumbers is false', async () => {
|
||||
const mockFile = createMockTFile('test.md');
|
||||
const content = '# Test Note\n\nContent here.';
|
||||
|
||||
(PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile);
|
||||
mockVault.read = jest.fn().mockResolvedValue(content);
|
||||
|
||||
const result = await noteTools.readNote('test.md');
|
||||
const result = await noteTools.readNote('test.md', { withLineNumbers: false });
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
// Now returns JSON even with default options
|
||||
// Returns JSON with raw content when line numbers disabled
|
||||
const parsed = JSON.parse(result.content[0].text);
|
||||
expect(parsed.content).toBe(content);
|
||||
expect(parsed.wordCount).toBe(5); // Test Note Content here
|
||||
});
|
||||
|
||||
it('should return numbered lines when withLineNumbers is true', async () => {
|
||||
it('should return numbered lines by default', async () => {
|
||||
const mockFile = createMockTFile('test.md', {
|
||||
ctime: 1000,
|
||||
mtime: 2000,
|
||||
@@ -215,7 +216,7 @@ describe('NoteTools', () => {
|
||||
(PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile);
|
||||
mockVault.read = jest.fn().mockResolvedValue(content);
|
||||
|
||||
const result = await noteTools.readNote('test.md', { withLineNumbers: true });
|
||||
const result = await noteTools.readNote('test.md');
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
const parsed = JSON.parse(result.content[0].text);
|
||||
@@ -225,7 +226,7 @@ describe('NoteTools', () => {
|
||||
expect(parsed.wordCount).toBe(6); // # Title Paragraph text More text
|
||||
});
|
||||
|
||||
it('should return versionId even without withLineNumbers', async () => {
|
||||
it('should return raw content when withLineNumbers is false', async () => {
|
||||
const mockFile = createMockTFile('test.md', {
|
||||
ctime: 1000,
|
||||
mtime: 2000,
|
||||
@@ -236,13 +237,52 @@ describe('NoteTools', () => {
|
||||
(PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile);
|
||||
mockVault.read = jest.fn().mockResolvedValue(content);
|
||||
|
||||
const result = await noteTools.readNote('test.md');
|
||||
const result = await noteTools.readNote('test.md', { withLineNumbers: false });
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
const parsed = JSON.parse(result.content[0].text);
|
||||
expect(parsed.content).toBe('# Test');
|
||||
expect(parsed.totalLines).toBeUndefined();
|
||||
expect(parsed.versionId).toBe('AXrGSV5GxqntccmzWCNwe7'); // SHA-256 hash of "2000-100"
|
||||
});
|
||||
|
||||
it('should return numbered lines in parseFrontmatter path by default', async () => {
|
||||
const mockFile = createMockTFile('test.md', {
|
||||
ctime: 1000,
|
||||
mtime: 2000,
|
||||
size: 100
|
||||
});
|
||||
const content = '---\ntitle: Test\n---\n\nContent here';
|
||||
|
||||
(PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile);
|
||||
mockVault.read = jest.fn().mockResolvedValue(content);
|
||||
|
||||
const result = await noteTools.readNote('test.md', { parseFrontmatter: true });
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
const parsed = JSON.parse(result.content[0].text);
|
||||
expect(parsed.content).toBe('1→---\n2→title: Test\n3→---\n4→\n5→Content here');
|
||||
expect(parsed.totalLines).toBe(5);
|
||||
});
|
||||
|
||||
it('should return raw content in parseFrontmatter path when withLineNumbers is false', async () => {
|
||||
const mockFile = createMockTFile('test.md', {
|
||||
ctime: 1000,
|
||||
mtime: 2000,
|
||||
size: 100
|
||||
});
|
||||
const content = '---\ntitle: Test\n---\n\nContent here';
|
||||
|
||||
(PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile);
|
||||
mockVault.read = jest.fn().mockResolvedValue(content);
|
||||
|
||||
const result = await noteTools.readNote('test.md', { parseFrontmatter: true, withLineNumbers: false });
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
const parsed = JSON.parse(result.content[0].text);
|
||||
expect(parsed.content).toBe(content);
|
||||
expect(parsed.totalLines).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('createNote', () => {
|
||||
|
||||
Reference in New Issue
Block a user