Added babel parser support for javascript parsing #72
Conversation
|
|
||
| Parser.extend(stage3).parse(script, ACORN_OPTIONS); | ||
| break; | ||
| case 'babel': |
| return getGettextEntriesFromScript(script, 'acorn'); | ||
| } catch (e) { | ||
| return getGettextEntriesFromScript(script, 'babel'); | ||
| } |
There was a problem hiding this comment.
I'd very much rather have 2 sub-functions, one that parses with babel, another that parses with acorn, and that you call them instead of having the function calling itself with different arguments.
Also, in the switch statemnt you could just leave this calling of the two parsers as default instead of using the 'auto' keyword :)
| function extractStringsFromJavascript(filename, script) { | ||
| const gettextEntries = getGettextEntriesFromScript(script); | ||
| function extractStringsFromJavascript(filename, script, parser = 'auto') { | ||
| const gettextEntries = getGettextEntriesFromScript(script, parser); |
There was a problem hiding this comment.
I think you could avoid adding a parser argument to getGettextEntriesFromScript and just directly call one or both of the parsing submethods (let's call them parseJSGettextWithAcorn() and parseJSGettextWithBabel() directly here. It would make a clearer separation of abstraction levels.
| case 'auto': | ||
| try { | ||
| return getGettextEntriesFromScript(script, 'acorn'); | ||
| } catch (e) { |
There was a problem hiding this comment.
you should log something so that the user sees that we just switched the parser.
| @@ -30,7 +30,7 @@ yarn add --dev easygettext | |||
| ##### HTML token extraction | |||
|
|
|||
| Simply invoke the tool on the templates you want to extract a POT dictionary template from. | |||
| The optional '--ouput' argument enables you to directly output to a file. | |||
| The optional '--output' argument enables you to directly output to a file. | |||
There was a problem hiding this comment.
we should document somewhere that we now support something more, don't you think ?
|
Exactly. I d'ont have time to do it though, but feel free since I doubt @ziemenz has got time enough on their hands for that :) |
As commented on [Polyconseil#72](Polyconseil#72)
|
Heads up @vperron @ziemenz , I'm currently working on rebasing this PR to version |
|
Yes, one of the original design choices was to keep the CLI part as simple as possible so that it does not need so much testing. The issue being that testing it requires some integration testing to run external processes and collect the output, which isn't an easy thing to do and automate. |
|
You are totally right to remind it to me ! I'll take a look into it asap. I
don't have a computer here though.
…On Fri, Oct 2, 2020, 10:57 Sven Parker ***@***.***> wrote:
@vperron <https://github.com/vperron> I hate to be a reminder for open
source projects, but now that a month has passed, have you had time to look
over the PR #92 <#92> yet?
It is supposed to take all of your suggestions in account.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#72 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAVUKS3TNWRHC6PW5WGTBDSIWIWPANCNFSM4J232ZJQ>
.
|
|
ASAP meaning tomorrow, I'm still on vacation with no computer today ^^ |
|
Done ! @Knogobert I'll make you a permanent contributor after your rebase & my merge ^^ |
|
Thanks @vperron , hope you had a nice vacation! 🌴 I'll fix those issues you mentioned before letting you merge |
As commented on [Polyconseil#72](Polyconseil#72)
As commented on [Polyconseil#72](Polyconseil#72)
|
Thanks a lot @Knogobert , I made you a contributor permanently and am closing this PR now ! |
Added new parameter for cli
--parser(autoby default)When parser is
autogettext-extracttry parse with useacornparser. Ifacornfinished with error, then try parse withbabelparser with using project config(e.g @babel/plugin-proposal-optional-chaining)