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.

Sign up for GitHub

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

Jump to bottom

fix: support multiple diagnostic in one line #699

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: support multiple diagnostic in one line #699

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fcfce9a
fix: support multiple diagnostic in one line
Aug 15, 2022
130e950
chore: use matchAll && update vscode engine version for code-actions-…
Aug 16, 2022
6cdd9e1
style: use tab for indent
Aug 17, 2022
20f5372
fix: escape reg && bump @types/vscode
Aug 18, 2022
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
20 changes: 12 additions & 8 deletions code-actions-sample/src/diagnostics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,25 @@ export function refreshDiagnostics(doc: vscode.TextDocument, emojiDiagnostics: v

for (let lineIndex = 0; lineIndex < doc.lineCount; lineIndex++) {
const lineOfText = doc.lineAt(lineIndex);
if (lineOfText.text.includes(EMOJI)) {
diagnostics.push(createDiagnostic(doc, lineOfText, lineIndex));
const regex = RegExp(EMOJI, 'g');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use new

Also this should escape special regex character in EMOJI in case someone modifies the code: https://stackoverflow.com/questions/3446170/escape-string-for-use-in-javascript-regex

let arr;
while ((arr = regex.exec(lineOfText.text)) !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you also update minimum VS Code engine from 1.32 to at least 1.40 you can use matchAll, which I think looks cleaner.

Suggested change
let arr;
while ((arr = regex.exec(lineOfText.text)) !== null) {
const matches = lineOfText.text.matchAll(regex);
for (const arr of matches) {

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. I do it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation looks off for the inner for.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, it look that because I use spaces for indent

diagnostics.push(createDiagnostic(doc, lineOfText, lineIndex, arr.index));
}
}

emojiDiagnostics.set(doc.uri, diagnostics);
}

function createDiagnostic(doc: vscode.TextDocument, lineOfText: vscode.TextLine, lineIndex: number): vscode.Diagnostic {
function createDiagnostic(doc: vscode.TextDocument, lineOfText: vscode.TextLine, lineIndex: number, startTextIndex: number): vscode.Diagnostic {
// find where in the line of that the 'emoji' is mentioned
const index = lineOfText.text.indexOf(EMOJI);

// create range that represents, where in the document the word is
const range = new vscode.Range(lineIndex, index, lineIndex, index + EMOJI.length);

const range = new vscode.Range(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting into multiple lines is not a worthwhile change IMO

lineIndex,
startTextIndex,
lineIndex,
startTextIndex + EMOJI.length
);
const diagnostic = new vscode.Diagnostic(range, "When you say 'emoji', do you want to find out more?",
vscode.DiagnosticSeverity.Information);
diagnostic.code = EMOJI_MENTION;
Expand All @@ -64,4 +68,4 @@ export function subscribeToDocumentChanges(context: vscode.ExtensionContext, emo
vscode.workspace.onDidCloseTextDocument(doc => emojiDiagnostics.delete(doc.uri))
);

}
}