Skip to content

Commit

Permalink
Merge pull request #195 from mjbvz/vscode-228085
Browse files Browse the repository at this point in the history
Support spaces in angle bracket link fragments
  • Loading branch information
mjbvz authored Oct 29, 2024
2 parents de90fd7 + 138c21d commit 955d899
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 41 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## 0.5.0-alpha.8 — Unreleased
- Fix incorrect detection of a multiline link definition #192
- Support spaces in angle brackets inside links: `[text](<#heading with spaces>)`

## 0.5.0-alpha.7 — July 25, 2024
- Strip markup from header document symbols. This makes them more readable.

Expand Down
2 changes: 1 addition & 1 deletion src/languageFeatures/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class MdDefinitionProvider {
}

const toc = await this.#tocProvider.get(resolvedResource);
return toc?.lookup(sourceLink.href.fragment)?.headerLocation;
return toc?.lookupByFragment(sourceLink.href.fragment)?.headerLocation;
}

#getDefinitionOfRef(ref: string, allLinksInFile: readonly MdLink[]) {
Expand Down
11 changes: 7 additions & 4 deletions src/languageFeatures/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as lsp from 'vscode-languageserver-protocol';
import { URI } from 'vscode-uri';
import { LsConfiguration } from '../config';
import { ILogger, LogLevel } from '../logging';
import { MdTableOfContentsProvider } from '../tableOfContents';
import { MdTableOfContentsProvider, TableOfContents } from '../tableOfContents';
import { HrefKind, InternalHref, LinkDefinitionSet, MdLink, MdLinkDefinition, MdLinkKind, MdLinkSource, ReferenceLinkMap } from '../types/documentLink';
import { translatePosition } from '../types/position';
import { modifyRange } from '../types/range';
Expand Down Expand Up @@ -224,12 +224,11 @@ export class DiagnosticComputer {

const diagnostics: lsp.Diagnostic[] = [];
for (const link of links) {

if (link.href.kind === HrefKind.Internal
&& link.source.hrefText.startsWith('#')
&& isSameResource(link.href.path, getDocUri(doc))
&& link.href.fragment
&& !toc.lookup(link.href.fragment)
&& !tocLookupByLink(toc, { source: link.source, fragment: link.href.fragment })
) {
// Don't validate line number links
if (parseLocationInfoFromFragment(link.href.fragment)) {
Expand Down Expand Up @@ -408,7 +407,7 @@ export class DiagnosticComputer {
continue;
}

if (!toc?.lookup(link.fragment) && !this.#isIgnoredLink(options, link.source.pathText) && !this.#isIgnoredLink(options, link.source.hrefText)) {
if (!(toc && tocLookupByLink(toc, link)) && !this.#isIgnoredLink(options, link.source.pathText) && !this.#isIgnoredLink(options, link.source.hrefText)) {
const range = (link.source.fragmentRange && modifyRange(link.source.fragmentRange, translatePosition(link.source.fragmentRange.start, { characterDelta: -1 }), undefined)) ?? link.source.hrefRange;
diagnostics.push({
code: DiagnosticCode.link_noSuchHeaderInFile,
Expand Down Expand Up @@ -667,3 +666,7 @@ export class DiagnosticsManager extends Disposable implements IPullDiagnosticsMa
this.#linkWatcher.deleteDocument(uri);
}
}

function tocLookupByLink(toc: TableOfContents, link: { readonly source: MdLinkSource; readonly fragment: string; }) {
return link.source.isAngleBracketLink ? toc.lookupByHeading(link.fragment) : toc.lookupByFragment(link.fragment);
}
4 changes: 2 additions & 2 deletions src/languageFeatures/documentHighlights.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class MdDocumentHighlightProvider {
const docUri = getDocUri(document);
for (const link of links) {
if (link.href.kind === HrefKind.Internal
&& toc.lookup(link.href.fragment) === header
&& toc.lookupByFragment(link.href.fragment) === header
&& link.source.fragmentRange
&& isSameResource(link.href.path, docUri)
) {
Expand Down Expand Up @@ -106,7 +106,7 @@ export class MdDocumentHighlightProvider {
const fragment = href.fragment.toLowerCase();

if (isSameResource(targetDoc, getDocUri(document))) {
const header = toc.lookup(fragment);
const header = toc.lookupByFragment(fragment);
if (header) {
yield { range: header.headerLocation.range, kind: lsp.DocumentHighlightKind.Write };
}
Expand Down
2 changes: 1 addition & 1 deletion src/languageFeatures/documentLinks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ export class MdLinkProvider extends Disposable {

if (doc) {
const toc = await this.#tocProvider.getForContainingDoc(doc, token);
const entry = toc.lookup(linkFragment);
const entry = toc.lookupByFragment(linkFragment);
if (entry) {
return { kind: 'file', uri: URI.parse(entry.headerLocation.uri), position: entry.headerLocation.range.start, fragment: linkFragment };
}
Expand Down
2 changes: 1 addition & 1 deletion src/languageFeatures/references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export class MdReferencesProvider extends Disposable {

if (resolvedResource && this.#isMarkdownPath(resolvedResource) && sourceLink.href.fragment && sourceLink.source.fragmentRange && rangeContains(sourceLink.source.fragmentRange, triggerPosition)) {
const toc = await this.#tocProvider.get(resolvedResource);
const entry = toc?.lookup(sourceLink.href.fragment);
const entry = toc?.lookupByFragment(sourceLink.href.fragment);
if (entry) {
references.push({
kind: MdReferenceKind.Header,
Expand Down
9 changes: 7 additions & 2 deletions src/tableOfContents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,13 @@ export class TableOfContents {
this.#slugifier = slugifier;
}

public lookup(fragment: string): TocEntry | undefined {
const slug = this.#slugifier.fromFragment(fragment);
public lookupByFragment(fragmentText: string): TocEntry | undefined {
const slug = this.#slugifier.fromFragment(fragmentText);
return this.entries.find(entry => entry.slug.equals(slug));
}

public lookupByHeading(text: string): TocEntry | undefined {
const slug = this.#slugifier.fromHeading(text);
return this.entries.find(entry => entry.slug.equals(slug));
}
}
Expand Down
18 changes: 18 additions & 0 deletions src/test/diagnostic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,24 @@ suite('Diagnostic Computer', () => {
const diagnostics = await getComputedDiagnostics(store, doc, workspace, {});
assertDiagnosticsEqual(diagnostics, []);
}));

test('Should support angle bracket links with spaces in fragment', withStore(async (store) => {
const doc1 = new InMemoryDocument(workspacePath('doc space1.md'), joinLines(
`[i](<doc space2.md>)`,
`[i](<doc space2.md#fr agment>)`,
`[i](<#fr agment>)`,
``,
'# FR agment',

));
const doc2 = new InMemoryDocument(workspacePath('doc space2.md'), joinLines(
'# FR agment',
));
const workspace = new InMemoryWorkspace([doc1, doc2]);

const diagnostics = await getComputedDiagnostics(store, doc1, workspace, {});
assertDiagnosticsEqual(diagnostics, []);
}));
});


Expand Down
22 changes: 15 additions & 7 deletions src/test/documentLinks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,43 +495,43 @@ suite('Link computer', () => {
]);
});

test('Should find link only within angle brackets.', async () => {
test('Should find link only within angle brackets', async () => {
const links = await getLinksForText(joinLines(
`[link](<path>)`
));
assertLinksEqual(links, [lsp.Range.create(0, 8, 0, 12)]);
});

test('Should find link within angle brackets even with link title.', async () => {
test('Should find link within angle brackets even with link title', async () => {
const links = await getLinksForText(joinLines(
`[link](<path> "test title")`
));
assertLinksEqual(links, [lsp.Range.create(0, 8, 0, 12)]);
});

test('Should find link within angle brackets even with surrounding spaces.', async () => {
test('Should find link within angle brackets even with surrounding spaces', async () => {
const links = await getLinksForText(joinLines(
`[link]( <path> )`
));
assertLinksEqual(links, [lsp.Range.create(0, 9, 0, 13)]);
});

test('Should find link within angle brackets for image hyperlinks.', async () => {
test('Should find link within angle brackets for image hyperlinks', async () => {
const links = await getLinksForText(joinLines(
`![link](<path>)`
));
assertLinksEqual(links, [lsp.Range.create(0, 9, 0, 13)]);
});

test('Should find link with spaces in angle brackets for image hyperlinks with titles.', async () => {
test('Should find link with spaces in angle brackets for image hyperlinks with titles', async () => {
const links = await getLinksForText(joinLines(
`![link](< path > "test")`
));
assertLinksEqual(links, [lsp.Range.create(0, 9, 0, 15)]);
});


test('Should not find link due to incorrect angle bracket notation or usage.', async () => {
test('Should not find link due to incorrect angle bracket notation or usage', async () => {
const links = await getLinksForText(joinLines(
`[link](<path )`,
`[link](<> path>)`,
Expand All @@ -540,14 +540,22 @@ suite('Link computer', () => {
assertLinksEqual(links, []);
});

test('Should find link within angle brackets even with space inside link.', async () => {
test('Should find link within angle brackets even with space inside link', async () => {
const links = await getLinksForText(joinLines(
`[link](<pa th>)`
));

assertLinksEqual(links, [lsp.Range.create(0, 8, 0, 13)]);
});

test('Should find link within angle brackets with spaces in fragment', async () => {
const links = await getLinksForText(joinLines(
`[link](<pa th#fr agment>)`
));

assertLinksEqual(links, [lsp.Range.create(0, 8, 0, 23)]);
});

test('Should find links with titles', async () => {
const links = await getLinksForText(joinLines(
`[link](<no such.md> "text")`,
Expand Down
46 changes: 23 additions & 23 deletions src/test/tableOfContents.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ suite('Table of contents', () => {
const doc = new InMemoryDocument(testFileName, '');
const provider = await createToc(doc);

assert.strictEqual(provider.lookup(''), undefined);
assert.strictEqual(provider.lookup('foo'), undefined);
assert.strictEqual(provider.lookupByFragment(''), undefined);
assert.strictEqual(provider.lookupByFragment('foo'), undefined);
});

test('Lookup should not return anything for document with no headers', async () => {
Expand All @@ -36,10 +36,10 @@ suite('Table of contents', () => {
));
const provider = await createToc(doc);

assert.strictEqual(provider.lookup(''), undefined);
assert.strictEqual(provider.lookup('foo'), undefined);
assert.strictEqual(provider.lookup('a'), undefined);
assert.strictEqual(provider.lookup('b'), undefined);
assert.strictEqual(provider.lookupByFragment(''), undefined);
assert.strictEqual(provider.lookupByFragment('foo'), undefined);
assert.strictEqual(provider.lookupByFragment('a'), undefined);
assert.strictEqual(provider.lookupByFragment('b'), undefined);
});

test('Lookup should return basic #header', async () => {
Expand All @@ -51,15 +51,15 @@ suite('Table of contents', () => {
const provider = await createToc(doc);

{
const entry = provider.lookup('a');
const entry = provider.lookupByFragment('a');
assert.ok(entry);
assert.strictEqual(entry!.line, 0);
}
{
assert.strictEqual(provider.lookup('x'), undefined);
assert.strictEqual(provider.lookupByFragment('x'), undefined);
}
{
const entry = provider.lookup('c');
const entry = provider.lookupByFragment('c');
assert.ok(entry);
assert.strictEqual(entry!.line, 2);
}
Expand All @@ -72,9 +72,9 @@ suite('Table of contents', () => {
));
const provider = await createToc(doc);

assert.strictEqual((provider.lookup('fOo'))!.line, 0);
assert.strictEqual((provider.lookup('foo'))!.line, 0);
assert.strictEqual((provider.lookup('FOO'))!.line, 0);
assert.strictEqual((provider.lookupByFragment('fOo'))!.line, 0);
assert.strictEqual((provider.lookupByFragment('foo'))!.line, 0);
assert.strictEqual((provider.lookupByFragment('FOO'))!.line, 0);
});

test('should handle special characters #44779', async () => {
Expand All @@ -84,7 +84,7 @@ suite('Table of contents', () => {
));
const provider = await createToc(doc);

assert.strictEqual((provider.lookup('indentação'))!.line, 0);
assert.strictEqual((provider.lookupByFragment('indentação'))!.line, 0);
});

test('should handle special characters 2, #48482', async () => {
Expand All @@ -94,7 +94,7 @@ suite('Table of contents', () => {
));
const provider = await createToc(doc);

assert.strictEqual((provider.lookup('инструкция---делай-раз-делай-два'))!.line, 0);
assert.strictEqual((provider.lookupByFragment('инструкция---делай-раз-делай-два'))!.line, 0);
});

test('should handle special characters 3, #37079', async () => {
Expand All @@ -109,12 +109,12 @@ suite('Table of contents', () => {

const provider = await createToc(doc);

assert.strictEqual((provider.lookup('header-2'))!.line, 0);
assert.strictEqual((provider.lookup('header-3'))!.line, 1);
assert.strictEqual((provider.lookup('Заголовок-2'))!.line, 2);
assert.strictEqual((provider.lookup('Заголовок-3'))!.line, 3);
assert.strictEqual((provider.lookup('Заголовок-header-3'))!.line, 4);
assert.strictEqual((provider.lookup('Заголовок'))!.line, 5);
assert.strictEqual((provider.lookupByFragment('header-2'))!.line, 0);
assert.strictEqual((provider.lookupByFragment('header-3'))!.line, 1);
assert.strictEqual((provider.lookupByFragment('Заголовок-2'))!.line, 2);
assert.strictEqual((provider.lookupByFragment('Заголовок-3'))!.line, 3);
assert.strictEqual((provider.lookupByFragment('Заголовок-header-3'))!.line, 4);
assert.strictEqual((provider.lookupByFragment('Заголовок'))!.line, 5);
});

test('Lookup should support suffixes for repeated headers', async () => {
Expand All @@ -126,17 +126,17 @@ suite('Table of contents', () => {
const provider = await createToc(doc);

{
const entry = provider.lookup('a');
const entry = provider.lookupByFragment('a');
assert.ok(entry);
assert.strictEqual(entry!.line, 0);
}
{
const entry = provider.lookup('a-1');
const entry = provider.lookupByFragment('a-1');
assert.ok(entry);
assert.strictEqual(entry!.line, 1);
}
{
const entry = provider.lookup('a-2');
const entry = provider.lookupByFragment('a-2');
assert.ok(entry);
assert.strictEqual(entry!.line, 2);
}
Expand Down

0 comments on commit 955d899

Please sign in to comment.