[FEAT] Expand code Link#116
Conversation
samad-yar-khan
left a comment
samad-yar-khan
left a comment
There was a problem hiding this comment.
@VipinDevelops Thanks for the PR. This is an amazing contribution. Please go through the requested changes.
github/handlers/HandleLinks.ts
Outdated
| async function checkLines(content, url) { | ||
| const regex: RegExp = /(?:L(\d+)+-L(\d+)|L(\d+))/; | ||
| const match: RegExpMatchArray = url.match(regex); | ||
| if (match[2]) { | ||
| const endLine = parseInt(match[2]) - parseInt(match[1]); | ||
| const lines = new RegExp( | ||
| `(?:.*\n){${parseInt(match[1]) - 1}}(.*(?:\n.*){${endLine}})` | ||
| ); | ||
| const text = await content.match(lines); | ||
| return text[1]; | ||
| } else if (match[3]) { | ||
| const lines = new RegExp( | ||
| `(?:.*\n){${parseInt(match[3]) - 1}}(.*(?:\n.*){5})` | ||
| ); | ||
| const text = await content.match(lines); | ||
| return text[1]; | ||
| } else { | ||
| const lines = new RegExp(`(?:.*\n){0}(.*(?:\n.*){5})`); | ||
| const text = await content.match(lines); | ||
| return text[1]; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
@VipinDevelops Although this code might be functional, its readability and naming should be improved. The function name should indicate what it does.
One good way to improve code readability is to add unit tests for these complex pure function. The test serve as the documentation for that complex piece of code.
There was a problem hiding this comment.
So I should introduce and implement the Test for this using JEST or something ?
|
Hello @samad-yar-khan Sir i made all the requested changes in the PR |
github/enum/URLmodifications.ts
Outdated
| @@ -0,0 +1,5 @@ | |||
| export enum URLEnums { | |||
There was a problem hiding this comment.
@VipinDevelops What does this URLEnums stand for ? Can you make this more self explanatory, improve naming or add comments.
github/handlers/HandleLinks.ts
Outdated
| async function extractCodeSnippet(content: string, url: string): Promise<string> { | ||
| const lineRangeRegex: RegExp = /(?:L(\d+)+-L(\d+)|L(\d+))/; | ||
| const lineRangeMatch: RegExpMatchArray | null = url.match(lineRangeRegex); | ||
|
|
There was a problem hiding this comment.
@VipinDevelops The spacing does not match the rest of the code base.
| if (lineRangeMatch) { | ||
| return extractCodeSnippetByLineRange(content, lineRangeMatch); | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing some indentation
if (lineRangeMatch) {
return extractCodeSnippetByLineRange(content, lineRangeMatch);
}
samad-yar-khan
left a comment
samad-yar-khan
left a comment
There was a problem hiding this comment.
This is an amazing contribution to our project ! Thanks a lot @VipinDevelops
2 participants
Issue(s)
Closes #26
Acceptance Criteria fulfillment
This pull request focuses on fulfilling the acceptance criteria related to expanding code links .
The changes introduced in this PR involve the following steps:
checkPreMessageSentExtendfunction first verifies whether the received message contains a GitHub link.executePreMessageSentExtendfunction then checks if the identified link is a code link.handleCodeLinkfunction is responsible for adding an attachment to the message, thereby expanding the code link.Proposed changes (including videos or screenshots)
Screencast.from.2023-06-22.11-46-22.webm