Skip to content

Commit

Permalink
Handle local labels correctly
Browse files Browse the repository at this point in the history 
All labels are currently treated as unique global symbols and are
suggested as completions anywhere in the project.

Labels with a dot prefix are contextual to their preceding regular
label. The same local label name can be defined multiple times in
different contexts. It only makes sense to offer local labels as a
completion when in the relevant scope.

The simplest solution for all of this is to prefix local labels when
defining and looking up symbols, effectively name-spacing them to their
parent label.

e.g.
```
Main:
.example
```
would be stored internally as `Main.example`
  • Loading branch information
@grahambates
grahambates committed Jan 2, 2022
1 parent ca0be0f commit a99f480
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 21 deletions.
19 changes: 14 additions & 5 deletions src/completion.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,23 @@ export class M68kCompletionItemProvider implements vscode.CompletionItemProvider
}
const asmLine = new ASMLine(text, line);
if (range) {
// Extend range to include leading dot
if (line.text.charAt(range.start.character -1) === '.') {
let prefix = "";
if (line.text.charAt(range.start.character -1) === ".") {
// Extend range to include leading dot
range = new vscode.Range(
new vscode.Position(range.start.line, range.start.character - 1),
range.end
)
);
// Find previous global label
for (let i = range.start.line; i >= 0; i--) {
const match = document.lineAt(i).text.match(/^(\w+)\b/);
if (match) {
prefix = match[0];
break;
}
}
}
const word = document.getText(range);
const word = prefix + document.getText(range);
const isInComment = (range.intersection(asmLine.commentRange) !== undefined);
const isInInstruction = (range.intersection(asmLine.instructionRange) !== undefined);
if ((!isInComment) && ((!isInInstruction) || ((lastChar !== ".") && (!asmLine.instruction.includes("."))))) {
Expand Down Expand Up @@ -87,7 +96,7 @@ export class M68kCompletionItemProvider implements vscode.CompletionItemProvider
for (const [label, symbol] of labels.entries()) {
if (!labelsAdded.includes(label)) {
const kind = vscode.CompletionItemKind.Function;
const completion = new vscode.CompletionItem(label, kind);
const completion = new vscode.CompletionItem(label.substring(prefix.length), kind);
const filename = symbol.getFile().getUri().path.split("/").pop();
const line = symbol.getRange().start.line;
completion.detail = "label " + filename + ":" + line;
Expand Down
13 changes: 11 additions & 2 deletions src/hover.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,25 @@ export class M68kHoverProvider implements vscode.HoverProvider {
} else if (asmLine.dataRange && asmLine.dataRange.contains(position)) {
// get the word
let word = document.getWordRangeAtPosition(position);
let prefix = "";
if (word) {
// Extend range to include leading dot
if (line.text.charAt(word.start.character -1) === '.') {
// Extend range to include leading dot
word = new vscode.Range(
new vscode.Position(word.start.line, word.start.character - 1),
word.end
)
// Find previous global label
for (let i = word.start.line; i >= 0; i--) {
const match = document.lineAt(i).text.match(/^(\w+)\b/);
if (match) {
prefix = match[0];
break;
}
}
}
// Text to search in
let text = document.getText(word);
let text = prefix + document.getText(word);
let rendered = await this.renderWordHover(text.toUpperCase());
let renderedLine2 = null;
if (!rendered) {
Expand Down
10 changes: 8 additions & 2 deletions src/symbols.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,20 @@ export class SymbolFile {
}
const instruct = asmLine.instruction.toLowerCase();
if (asmLine.label.length > 0) {
const label = asmLine.label.replace(":", "");
let label = asmLine.label.replace(":", "");
const isLocal = label.indexOf(".") === 0;
if (isLocal) {
label = lastLabel?.getLabel() + label;
}
const s = new Symbol(label, this, asmLine.labelRange);
// Is this actually a macro definition in `<name> macro` syntax?
if (instruct.indexOf("macro") === 0) {
this.macros.push(s);
} else {
this.labels.push(s);
lastLabel = s;
if (!isLocal) {
lastLabel = s;
}
}
} else if (instruct.indexOf("macro") === 0) {
// Handle ` macro <name>` syntax
Expand Down
38 changes: 30 additions & 8 deletions src/test/completion.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,17 +118,39 @@ describe("Completion Tests", function () {
expect(elm.detail).to.be.equal("label tutorial.s:100");
expect(elm.label).to.be.equal("Main");
});
it("Should return a completion on a local label", async function () {
const cp = new M68kCompletionItemProvider(documentationManager, state.getDefinitionHandler(), await state.getLanguage());
it("Should return a completion on a local label in scope", async function () {
const document = new DummyTextDocument();
const position: Position = new Position(0, 11);
const position: Position = new Position(1, 8);
const tokenEmitter = new CancellationTokenSource();
document.addLine(" jsr .chkmo");
document.addLine("Example:");
document.addLine(" jsr .examp");
document.addLine(".example");
const definitionHandler = state.getDefinitionHandler();
await definitionHandler.scanFile(document.uri, document);
const cp = new M68kCompletionItemProvider(documentationManager, definitionHandler, await state.getLanguage());

const results = await cp.provideCompletionItems(document, position, tokenEmitter.token);
expect(results).to.not.be.undefined;
const elm = results[0];
expect(elm.detail).to.be.equal("label tutorial.s:127");
expect(elm.label).to.be.equal(".chkmouse");
expect(results).to.not.be.empty;
const elm = results.find(n => n.label === '.example')!;
expect(elm).to.not.be.empty;
expect(elm.detail).to.be.equal("label file.s:2");
expect(elm.label).to.be.equal(".example");
});
it("Should not return a completion on a local label out of scope", async function () {
const document = new DummyTextDocument();
const position: Position = new Position(1, 8);
const tokenEmitter = new CancellationTokenSource();
document.addLine("Example:");
document.addLine(" jsr .examp");
document.addLine("Example2:");
document.addLine(".example");
const definitionHandler = state.getDefinitionHandler();
await definitionHandler.scanFile(document.uri, document);
const cp = new M68kCompletionItemProvider(documentationManager, definitionHandler, await state.getLanguage());

const results = await cp.provideCompletionItems(document, position, tokenEmitter.token);
const elm = results.find(n => n.label === '.example')!;
expect(elm).to.be.undefined;
});
it("Should return a completion on an instruction", async function () {
const cp = new M68kCompletionItemProvider(documentationManager, state.getDefinitionHandler(), await state.getLanguage());
Expand Down
8 changes: 4 additions & 4 deletions src/test/dummy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ export class DummyTextDocument implements TextDocument {
lineCount = 0;
lines = new Array<TextLine>();
public addLine(line: string) {
const newLine = new DummyTextLine(line);
newLine.lineNumber = this.lineCount;
const newLine = new DummyTextLine(line, this.lineCount);
this.lines.push(newLine);
this.lineCount += 1;
}
Expand Down Expand Up @@ -145,9 +144,10 @@ export class DummyTextLine implements TextLine {
rangeIncludingLineBreak: Range = new Range(new Position(0, 0), new Position(0, 1));
firstNonWhitespaceCharacterIndex = 1;
isEmptyOrWhitespace = false;
constructor(text: string) {
constructor(text: string, lineNumber: number = 0) {
this.text = text;
this.range = new Range(new Position(0, 0), new Position(0, text.length));
this.range = new Range(new Position(lineNumber, 0), new Position(lineNumber, text.length));
this.lineNumber = lineNumber;
}
}

Expand Down

0 comments on commit a99f480

Please sign in to comment.