Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support spaces in angle bracket link fragments #195

Merged
merged 2 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading