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

Comments and Multi-line Highlight Support #68

Merged
merged 5 commits into from
Feb 27, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
46 changes: 24 additions & 22 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export function activate(context: vscode.ExtensionContext) {
}>();
highlighterNode.highlights.map(highlight =>
highlightsToRemove.push({
lineNumber: highlight.lineNumber,
lineNumber: highlight.startLine,
fileName: highlighterNode.document.fileName
})
);
Expand Down Expand Up @@ -197,7 +197,7 @@ export function activate(context: vscode.ExtensionContext) {
function highlightHandler() {
vscode.window
.showInputBox({ prompt: 'Enter a line number' })
.then(lineString => highlight(+(lineString || 0), 'self'));
.then(lineString => highlight('self', +(lineString || 0), +(lineString || 0)));
}

function unhighlightAllHandler() {
Expand Down Expand Up @@ -305,10 +305,10 @@ export function deactivate(): Thenable<void> {
return twitchChatClient.dispose();
}

function highlight(line: number, twitchUser: string) {
function highlight(twitchUser: string, startLine: number, endLine: number, fileName?: string, comment?: string) {
console.log(`highlight called.`);
if (!line) {
console.warn('A line number was not provided to unhighlight');
if (!startLine) {
console.warn('A line number was not provided to highlight');
return;
}

Expand All @@ -327,36 +327,36 @@ function highlight(line: number, twitchUser: string) {
if (
existingHighlighter &&
existingHighlighter.highlights.some(
h => h.twitchUser === twitchUser && h.lineNumber === line
h => h.twitchUser === twitchUser && h.startLine === startLine
)
) {
console.log(
`An existing highlight already exists for '${twitchUser}' on line '${line}'`
`An existing highlight already exists for '${twitchUser}' starting on line '${startLine}'`
);
return;
}

const range = getHighlightRange(line, doc);
const range = getHighlightRange(startLine, endLine, doc);
if (range.isEmpty) {
/**
* TODO: Maybe whisper to the end-user that the line requested is empty.
* Although whispers aren't gaurenteed to reach the end-user.
*/
console.log(`line #'${line}' is empty. Cancelled.`);
console.log(`line #'${startLine}' is empty. Cancelled.`);
return;
}

const decoration = {
range,
hoverMessage: `From @${twitchUser === 'self' ? 'You' : twitchUser}`
hoverMessage: `From @${twitchUser === 'self' ? 'You' : twitchUser}${comment !== undefined ? `: ${comment}` : ''}`
};

addHighlight(existingHighlighter, decoration, editor, line, twitchUser);
addHighlight(existingHighlighter, decoration, editor, startLine, endLine, twitchUser);
}

function unhighlight(line: number, fileName: string) {
function unhighlight(lineNumber: number, fileName?: string) {
console.log('unhighlight called.');
if (!line) {
if (!lineNumber) {
vscode.window.showWarningMessage(
'A line number was not provided to unhighlight.'
);
Expand Down Expand Up @@ -387,7 +387,7 @@ function unhighlight(line: number, fileName: string) {
currentDocumentFileName = existingHighlighter.editor.document.fileName;
}

removeHighlight(line, currentDocumentFileName);
removeHighlight(lineNumber, currentDocumentFileName);
}

// Listen for active text editor or document so we don't lose any existing highlights
Expand Down Expand Up @@ -429,19 +429,20 @@ function addHighlight(
existingHighlighter: Highlighter | undefined,
decoration: { range: vscode.Range; hoverMessage: string },
editor: vscode.TextEditor,
lineNumber: number,
startLine: number,
endLine: number,
twitchUser: string
) {
if (existingHighlighter) {
// We have a new decoration for a highlight with decorations already in a file
// Add the decoration (a.k.a. style range) to the existing highlight's decoration array
// Reapply decoration type for updated decorations array in this highlight
existingHighlighter.addHighlight(
new Highlight(decoration, lineNumber, twitchUser)
new Highlight(decoration, startLine, endLine, twitchUser)
);
} else {
const highlighter = new Highlighter(editor, [
new Highlight(decoration, lineNumber, twitchUser)
new Highlight(decoration, startLine, endLine, twitchUser)
]);
highlighters.push(highlighter);
}
Expand Down Expand Up @@ -473,17 +474,18 @@ function findHighlighter(fileName: string): Highlighter | undefined {
});
}

function getHighlightRange(line: number, doc: vscode.TextDocument) {
function getHighlightRange(startLine: number, endLine: number, doc: vscode.TextDocument) {
// prefix string with plus (+) to make string a number
// well at least that's what codephobia says :P
// const zeroIndexedLineNumber = +lineNumber - 1;
// note: doc.lineAt is zero based index so remember to always do -1 from input
const zeroIndexedLineNumber = line - 1;
let textLine = doc.lineAt(zeroIndexedLineNumber);
// const zeroIndexStartLineNumber = startLine - 1;
// const zeroIndexedEndLineNumber = endLine - 1;
let textLine = doc.lineAt(--endLine);
let textLineLength = textLine.text.length;
let range = new vscode.Range(
new vscode.Position(zeroIndexedLineNumber, 0),
new vscode.Position(zeroIndexedLineNumber, textLineLength)
new vscode.Position(--startLine, 0),
new vscode.Position(endLine, textLineLength)
);
return range;
}
Expand Down
7 changes: 4 additions & 3 deletions src/highlighter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ export class Highlighter {

getPickerDetails() {
return this.highlights.map(highlight => {
return `${this.editor.document.fileName}, ${highlight.lineNumber}`;
return `${this.editor.document.fileName}, ${highlight.startLine}`;
});
}

removeDecoration(lineNumber: number): Highlight[] {
const highlightIndex = this.highlights.findIndex(highlight => {
return highlight.lineNumber === lineNumber;
return highlight.startLine <= lineNumber && highlight.endLine >= lineNumber;
});
if (highlightIndex > -1) {
return this.highlights.splice(highlightIndex, 1);
Expand All @@ -43,7 +43,8 @@ export class Highlighter {
export class Highlight {
constructor(
public decoration: { range: vscode.Range },
public lineNumber: number,
public startLine: number,
public endLine: number,
public twitchUser?: string
) { }
}
19 changes: 12 additions & 7 deletions src/twitchChatClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,21 @@ export class TwitchChatClient {

/**
* Called when a highlight request is made from chat
* @param line The line number to highlight
* @param twitchUser The user that made the request
* @param startLine The line number to start the highlight on
* @param endLine The line number to end the highlight on
* @param fileName The `TextDocument`s filename to highlight in
* @param comments The comments to add to the decoration
* this.onHighlight(params.twitchUser, params.startLine, params.endLine, params.fileName, params.comments);
*/
public onHighlight?: (line: number, twitchUser: string) => void;

public onHighlight?: (twitchUser: string, startLine: number, endLine: number, fileName?: string, comments?: string) => void;
/**
* Called when an unhighlight request is made from chat
* @param line The line number to unhighlight
* @param fileName The TextDocument's filename to remove the highlight from
* @param lineNumber The line number to unhighlight, the entire highlight is removed if the lineNumber exists in the highlight range.
* @param fileName The `TextDocument`s filename to remove the highlight from
*/
public onUnhighlight?: (line: number, fileName: string) => void;
public onUnhighlight?: (lineNumber: number, fileName?: string) => void;
/**
* Called when the chat client is connecting
*/
Expand Down Expand Up @@ -196,14 +201,14 @@ export class TwitchChatClient {
this._languageClient.onNotification('highlight', (params: any) => {
console.log('highlight requested.', params);
if (this.onHighlight) {
this.onHighlight(params.line, params.twitchUser);
this.onHighlight(params.twitchUser, params.startLine, params.endLine, params.fileName, params.comment);
}
});

this._languageClient.onNotification('unhighlight', (params: any) => {
console.log('unhighlight requested.', params);
if (this.onUnhighlight) {
this.onUnhighlight(params.line, params.fileName);
this.onUnhighlight(params.endLine, params.fileName);
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/twitchHighlighterTreeView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class HighlighterNode extends vscode.TreeItem {
public getHighlights(): HighlighterNode[] {
const childrenNodes = new Array<HighlighterNode>();
this.highlights.forEach((highlight: Highlight) => {
const label = `Line: ${highlight.lineNumber}`;
const label = `Line: ${highlight.startLine}`;
const existingNode = childrenNodes.find(node => node.label === label);
if (existingNode) {
existingNode.highlights.push(highlight);
Expand All @@ -74,7 +74,7 @@ export class HighlighterNode extends vscode.TreeItem {
new HighlighterNode(label, this.document, [highlight], undefined, {
command: 'twitchHighlighter.gotoHighlight',
title: '',
arguments: [highlight.lineNumber, this.document]
arguments: [highlight.startLine, this.document]
})
);
}
Expand Down
101 changes: 48 additions & 53 deletions src/twitchLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,61 +80,56 @@ function onTtvChatMessage(channel: string, user: any, message: string) {
parseMessage(userName, message);
}

let highlighterCommands = ['!line', '!highlight'];
let highlightCommandUsed: string;

function parseMessage(userName: string, message: string) {
message = message.toLocaleLowerCase();
// Note: as RamblingGeek suggested might want to look into
// using switch instead of if/else for better performance
if (!isHighlightCommand(message)) {
return;
}

const chatMessageRawAction = message
.slice(highlightCommandUsed.length)
.trim();

const messageParts = chatMessageRawAction.split(' ');
if (messageParts.length === 0) {
// Example: !<command>
return;
}

const notificationType = messageParts[0].startsWith('!')
? 'unhighlight'
: 'highlight';
const lineNumber = messageParts[0].replace('!', '');
// Possible formats to support:
// !<command> <line number> <default to currently open file>
// !<command> <line number> <filename.ts>
// !<command> <line number> <filename.ts>
// !<command> <line number> <filename.ts>
// !<command> <line number> <filename.ts>
// !<command> !8 <filename.ts>
if (messageParts.length === 1) {
// Example: !<command> <line number>
connection.sendNotification(notificationType, {
line: +lineNumber,
twitchUser: userName
});
} else {
// Format Example: !<command> <line number> <filename.ts>
// Other Example: !<command> <line number> <filename.ts> <color>
connection.sendNotification(notificationType, {
line: +lineNumber,
filename: messageParts[1],
twitchUser: userName
});
}
}

function isHighlightCommand(message: string) {
return highlighterCommands.some(
(command: string): boolean => {
const comparison = message.startsWith(command.toLowerCase());
highlightCommandUsed = comparison ? command : '';
return comparison;
/**
* Regex pattern to verify the command is a highlight command
* groups the different sections of the command.
*
* See `https://regexr.com/48gf0` for my tests on the pattern.
*
* Matches:
*
* !line 5
* !line !5
* !line 5-10
* !line !5-15
* !line settings.json 5
Copy link
Owner

Choose a reason for hiding this comment

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

Haven't had a chance to run the code yet to test it but does this also support !line 5 settings.json? I only ask because it feels like that may be the natural/organic way to type/think about the usage. Do you agree? Might just be me who thinks that way though.

In general, how would we think/speak what we want to share? Something like "There's an issue on line 5 in your settings.json file"? Or "There's an issue in your settings.json file on line 5"? Should we support both or just one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right now it would not support !line 5 settings.json but I can adjust the regex pattern to support that flow. See https://regexr.com/48gf0 to test the pattern.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just pushed a new commit which supports @clarkio's suggestion

* !line settings.json !5
* !line settings.json 5-15
* !line settings.json !5-15
* !line settings.json 5 including a comment
* !line settings.json 5-15 including a comment
* !line settings.json 5 5 needs a comment
* !line 5 5 needs a comment
* !line 5-7 6 should be deleted
* !line settings.json 5-7 6 should be deleted
* !highlight 5
*
*/
const commandPattern = /\!(?:line|highlight) (?:((?:[\w]+)?\.[\w]{1,}) )?(\!)?(\d+)(?:-{1}(\d+))?(?: (.+))?/;

const cmdopts = commandPattern.exec(message);
if (!cmdopts) { return; }

const fileName: string = cmdopts[1];
const highlight: boolean = cmdopts[2] === undefined;
const startLine: number = +cmdopts[3];
const endLine: number = cmdopts[4] ? +cmdopts[4] : +cmdopts[3];
const comment: string | undefined = cmdopts[5];

// Ensure that the startLine is smaller than the endLine.
const vStartLine = endLine < startLine ? endLine : startLine;
const vEndLine = endLine < startLine ? startLine : endLine;

connection.sendNotification(
highlight ? 'highlight' : 'unhighlight',
{
twitchUser: userName,
startLine: vStartLine,
endLine: vEndLine,
fileName,
comment
}
);
}
Expand Down