-
Notifications
You must be signed in to change notification settings - Fork 2
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
Added findURLs, isValid, toTitleCase functions and their corresponding test files #13
base: main
Are you sure you want to change the base?
Conversation
In |
Updating the code and removing all the red errors for the above three functions
Importing findURLs, toTitleCase, validURL helper functions in index.ts
findURLs: The test file for this function involves various test cases which test whether the function returns the correct start and end indices for the correctly identified URLs, edge cases such as when the URL has a trailing or leading punctuation mark and returns empty array for strings without valid URLs or for invalid input formats. toTitleCase: It also includes the test file to validate the function's behavior. It first tests the function with some tests that would return valid, some edge cases such as single letters, all capitalized strings etc. and tests whether the appropriate error is thrown incase an input is invalid isValid: It also includes a test file to verify the function's correctness. The test file consists of many examples of URLs that would be considered valid, as well as invalid. |
Changed findURL and isValid functions, all three tests pass now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More comments
src/helpers/validators/findURL.ts
Outdated
@@ -2,19 +2,19 @@ | |||
* This function finds URLs within a given string and returns an array of | |||
* their locations. | |||
* @author Leisha Bhandari | |||
* @param block: The block of text to search for URLs | |||
* @param block The block of text to search for URLs | |||
* @returns Arrays where each of them contain the start and end index of the URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSDoc has to be right above the function. here, the constant urlRegex
is between the function and its documentation
* @returns Input string converted to title case | ||
*/ | ||
// List of lowercase words according to Chicago Manual of Style | ||
const lowerCaseWords: string[] = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this above so JSDoc isn't separated from function
src/helpers/validators/validURL.ts
Outdated
}; | ||
|
||
export default isValid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need endline
src/helpers/validators/findURL.ts
Outdated
// URLs within the block of text (Also takes care of Unicode characters) | ||
const urlRegex = /(https?:\/\/[^\s\u0000-\u001F.,]+[^\s\u0000-\u001F.,]?(?:\?[^ \s\u0000-\u001F]+)?(?:#[^\s\u0000-\u001F]+)?)/g; | ||
|
||
const findURLs = (block: string): { start: number, end: number }[] => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filename isn't named correctly (must match function name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: how hard would it be to add a feature where the URLs themselves are returned? Like this:
{ start: number, end: number, url: string }[]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be good to add a "unit" to the numbers. The unit here is index
so, like startIndex
and endIndex
src/helpers/validators/isValid.ts
Outdated
const parsed = new URL(url); | ||
|
||
// Checks that the URL's protocol is valid | ||
if (!['https:', 'http:', 'file:', 'ftp:'].includes(parsed.protocol)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only https and http
// Capitalize the first, last word, and any word in the input that is not a part of the lowercaseWords list | ||
return (index === 0 || index === words.length - 1 || !lowerCaseWords.includes(word) | ||
? `${word.charAt(0).toUpperCase()}${word.slice(1)}` | ||
: word); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One item per line
return (
(index === 0 || index === words.length - 1 || !lowerCaseWords.includes(word))
? `${word.charAt(0).toUpperCase()}${word.slice(1)}`
: word
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments. Super close!
Fixed the errors in all three functions and test cases, all tests pass
No description provided.