-
Notifications
You must be signed in to change notification settings - Fork 459
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
[EXTERNAL] Improved checking for import, moved removeComments #1765
base: master
Are you sure you want to change the base?
Conversation
Improved checking of imports and moved removeComments to separate function, clearly explaining that it's limited to javascript type of comments.
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.
Trying best to get the commit approved as it is really a very slow process.
} | ||
return { code: sanitizedCode, rawCode, path } | ||
} catch (error) { | ||
console.error(error) |
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.
the maintainer may replace the console.log(error) with fatal(error)
as fatal does precisely that plus whatever is required to be done on top of it
} | ||
|
||
const removeComments = (code) => { |
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.
the maintainer may replace this as const removeComments = (code, lang='JavaScript') =>
this helps in further direction to improve implementation
const removeComments = (code) => { | ||
// removes JS single line and multi-line comments only. Not for bash files etc. | ||
// for use with multiple file-types, I suggest writing a removeComments function with language-type as input and then handling accordingly | ||
return code.replace(/\/\*[\s\S]*?\*\/|\/\/.*/g, "").trim() |
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.
a condition could be added, something like if (lang === 'JavaScript') || (lang === 'JS') {
Improved checking of imports and moved removeComments to separate function, clearly explaining that it's limited to javascript type of comments.
Why?
Solution Overview
Implementation Details