Skip to content

Commit

Permalink
fixed source name lookup
Browse files Browse the repository at this point in the history
  • Loading branch information
ShaoYuan-Weng committed Aug 25, 2020
1 parent 293182c commit 5a66bfa
Showing 1 changed file with 40 additions and 13 deletions.
53 changes: 40 additions & 13 deletions src/definition_provider/sourceDefinitionProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import { isEnclosedWithinCodeBlock } from "../utils";
export class SourceDefinitionProvider implements DefinitionProvider {
private sourceMetaMap: SourceMetaMap = new Map();
private static readonly IS_SOURCE = /(source)[^}]*/;
private static readonly GET_SOURCE_NAME = /(?!['"])(\w+)(?=['"])/ig;
private static readonly HAS_SOURCE_NAME = /(?!['"])(\w+)(?=['"])/;
private static readonly GET_SOURCE_INFO = /(?!['"])(\w+)(?=['"])/g;

This comment has been minimized.

Copy link
@mdesmet

mdesmet Aug 25, 2020

Contributor

These regexes are also used in the autocompletes. maybe we need to put them somewhere where we can reuse them. Is there any other logic that is used in both cases?


provideDefinition(
document: TextDocument,
Expand All @@ -19,15 +20,27 @@ export class SourceDefinitionProvider implements DefinitionProvider {
const range = document.getWordRangeAtPosition(position, SourceDefinitionProvider.IS_SOURCE);
const word = document.getText(range);

if (range && word !== undefined && hover !== "source"
&& isEnclosedWithinCodeBlock(document, range)) {
const source = word.match(SourceDefinitionProvider.GET_SOURCE_NAME);
if (source) {
const definition = this.getSourceDefinition(source[0]);
resolve(definition);
return;
}
const linePrefix = document
.lineAt(position)
.text.substr(0, position.character);

if (!isEnclosedWithinCodeBlock(document, position) ||
!linePrefix.includes('source') ||
hover === 'source') { return undefined; }

const source = word.match(SourceDefinitionProvider.GET_SOURCE_INFO);
if (source === null || source === undefined) {
return undefined;
}

const sourceInfo = linePrefix.match(SourceDefinitionProvider.HAS_SOURCE_NAME) ? this.getTableName(source) : this.getSourceName(source);

if (sourceInfo) {
const definition = this.getSourceDefinition(sourceInfo.sourceName, sourceInfo.lookupItem);
resolve(definition);
return;
}

reject();
});
}
Expand All @@ -36,18 +49,32 @@ export class SourceDefinitionProvider implements DefinitionProvider {
this.sourceMetaMap = event.sourceMetaMap;
}

private getSourceDefinition(name: string): Definition | undefined {
const location = this.sourceMetaMap.get(name);
private getSourceName(source: RegExpMatchArray) {

This comment has been minimized.

Copy link
@mdesmet

mdesmet Aug 25, 2020

Contributor

Type the return value

return {
sourceName: source[0],
lookupItem: source[0]
};
}

private getTableName(source: RegExpMatchArray) {
return {
sourceName: source[0],
lookupItem: source[1]
};
}

private getSourceDefinition(sourceName: string, lookupItem: string): Definition | undefined {
const location = this.sourceMetaMap.get(sourceName);
if (location) {
const sourceFile: string = readFileSync(location.path).toString("utf8");
const sourceFileLines = sourceFile.split("\n");

for (let index = 0; index < sourceFileLines.length; index++) {
const currentLine = sourceFileLines[index];
if (currentLine.includes(name)) {
if (currentLine.includes(lookupItem)) {

This comment has been minimized.

Copy link
@mdesmet

mdesmet Aug 25, 2020

Contributor

it's possible that a name of the source will be the same of the table, or that a the another table exists that contains the searched value. eg. TABLE_1 and TABLE_12. So we need to match the actual value instead of just checking that it 's contained.

This comment has been minimized.

Copy link
@mdesmet

mdesmet Aug 26, 2020

Contributor

In hindsight the cost of fixing this doesn't weigh up to its benefits

return new Location(
Uri.file(location.path),
new Position(index, currentLine.indexOf(name))
new Position(index, currentLine.indexOf(lookupItem))
);
}
}
Expand Down

0 comments on commit 5a66bfa

Please sign in to comment.