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

Add support for translation extraction from VueJs templates #178

Merged
merged 10 commits into from
May 14, 2018
Merged

Add support for translation extraction from VueJs templates #178

merged 10 commits into from
May 14, 2018

Conversation

briedis
Copy link
Contributor

@briedis briedis commented Apr 29, 2018

How it works - VueJs tempaltes basically are valid HTML. We extract template part (then extract dynamic attributes which are JS expressions and extract expressions from within template elements) and script part and parse them both as a regular JS.

@briedis
Copy link
Contributor Author

briedis commented Apr 29, 2018

Checking the tests...

JsFunctionsScanner now normalizes newline characters.
@briedis
Copy link
Contributor Author

briedis commented Apr 29, 2018

Will try to fix this on a linux machine. Tests on mac seem fine, but somehow CI converts something differently.

@briedis
Copy link
Contributor Author

briedis commented May 2, 2018

@oscarotero Everything looks fine, can you consider merging this, please? :)


return $functions;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this class is not necessary, the lineOffset could be added as an option for the JsFunctionScanner, so it can be reused by other javascript extractors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I'll make the changes after my vacation (14th of May). Hope this can sit here until then.

@oscarotero
Copy link
Member

Hi. Sorry for the delay.
I think your approach is a good start but has room for improvement. For example I wouldn't use the DOM library because there's no need to treat differently scripts, attributes and text content. I'd try to isolate the javascript code from the rest of the html, using a loop char by char, removing all code but javascript and spaces and line breaks. For example, this code:

<template>
    {{ __('Hello world') }}
    <p :attribute="__('One text')">Paragraph content {{ __('text') }}</p>
</template>

<script>
    console.log(__('Other text'));
</script>

would be converted to this:

     __('Hello world') 
    __('One text')   __('text') 



    console.log(__('Other text'));

Then, just use the jsCodeExtractor and that's it. So it could be reused by other future extractors like angular, react, etc.

Anyway, if you need this now, I can merge it (after the requested change is made) and we can improve this in future versions.

@briedis
Copy link
Contributor Author

briedis commented May 2, 2018

I'm not sure about the manual parsing of the template part, sounds like a lot of work and a lot of opportunities to mess things up if there's a mistake in the symbol counter. Especially for attributes where quotes can be escaped etc. Are you worried about the performance?

@oscarotero
Copy link
Member

It's not about performance, I think it's simpler than creating three fake javascripts, fixing line number issues, etc. All that things looks a bit hacky to me.

Manual parsing is what is used in JsFunctionScanner and works great. In html I guess just controlling the open/close tags, open/close attributes, html comments and open/close {{expressions}} should be enought. That also prevents issues with invalid html.

I dont think it's a lot of work (or maybe yes, you never know before start) 😄

@briedis
Copy link
Contributor Author

briedis commented May 4, 2018

Without DOM parsing, I imagine extracting attributes would be hard taking in account all quotes, escaping :attr="blabla('foo " bar ')" etc. HTML seems just too irregular. Handling it as DOM seems more safe.

@oscarotero
Copy link
Member

Ok, if the current implementation works fine, I'll merge it once the requesting change is made.
In a future, it can be improved.

@briedis
Copy link
Contributor Author

briedis commented May 14, 2018

VueJsFunctionScanner removed.

@oscarotero oscarotero merged commit 94aadf5 into php-gettext:master May 14, 2018
@oscarotero
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants