FireWire Posted November 25, 2020 Author Share Posted November 25, 2020 Interesting, I haven't seen that on my side but will update! Link to comment Share on other sites More sharing options...
FireWire Posted November 25, 2020 Author Share Posted November 25, 2020 18 minutes ago, adrian said: @FireWire - just installed and on going to the Translation page, I get these errors: PHP Warning: file_get_contents(/site/modules/Fluency/fluency_templates/el_h1.tpl.html): failed to open stream: No such file or directory in .../Fluency/classes/FluencyTools.class.php:31 PHP Warning: file_get_contents(/site/modules/Fluency/fluency_templates/el_p.tpl.html): failed to open stream: No such file or directory in .../Fluency/classes/FluencyTools.class.php:31 I installed via the URL to the zip file and the problem is that it resulted in two "Fluency" directories such that the path is actually: /var/www/dev.grief.coach/site/modules/Fluency/Fluency/fluency_templates/el_p.tpl.html Once I moved those files and did a modules > refresh it started working as expected. I'm not able to replicate this on my end- just to clarify, are you getting two nested Fluency directories in the .zip file? I haven't seen that before. Link to comment Share on other sites More sharing options...
adrian Posted November 25, 2020 Share Posted November 25, 2020 Looking at the repo (https://gitlab.com/SkyLundy/fluency-processwire), it seems pretty clear why it's happening - you have all the files stored inside a "Fluency" folder. When PW installs from the zip file, it extracts everything into a higher level Fluency directory, which is why there end up being two. Take a look at any other PW module and you'll see that the main files are at the top level. 1 Link to comment Share on other sites More sharing options...
FireWire Posted November 25, 2020 Author Share Posted November 25, 2020 Oh now I get it. I misunderstood when you said you installed from a URL. I don't make use of that (for some reason, habit?) so I've never run into that before and it didn't hit me. I'll restructure the repo and push that out. Thanks for the heads up. edit: Doneski. Repo has been updated. 1 Link to comment Share on other sites More sharing options...
adrian Posted November 25, 2020 Share Posted November 25, 2020 Great - thanks! I have another request for you - can you make it possible to not translate hanna codes, eg anything inside this: [[don't_translate]] - you'll want to make sure to find out what the open and close tags are for Hanna in each PW install, but that's easy. Does that make sense? Thanks again - this is really awesome!!! 1 Link to comment Share on other sites More sharing options...
adrian Posted November 25, 2020 Share Posted November 25, 2020 Or perhaps you could add a little more flexibility by allowing the "Global Non-translated strings" option to support regexes so we could exclude things like hanna codes and others as needed? 1 Link to comment Share on other sites More sharing options...
FireWire Posted November 25, 2020 Author Share Posted November 25, 2020 16 minutes ago, adrian said: Great - thanks! I have another request for you - can you make it possible to not translate hanna codes, eg anything inside this: [[don't_translate]] - you'll want to make sure to find out what the open and close tags are for Hanna in each PW install, but that's easy. Does that make sense? Thanks again - this is really awesome!!! 8 minutes ago, adrian said: Or perhaps you could add a little more flexibility by allowing the "Global Non-translated strings" option to support regexes so we could exclude things like hanna codes and others as needed? These are stellar ideas. I can add the hanna code exclusion as a config screen setting. I'll put that on the roadmap. The regex idea is also great. I think adding them both will keep it flexible and friendly (regex... not always friendly haha) Really glad you are enjoying the module. I want to get more feedback from people using it in production before pushing it out of alpha and adding it to the modules directory so when you get a chance to use it more I welcome your feedback and any bugs you come across. We're using it production for a website launching on Friday and it's done well for us but more data is always better! Link to comment Share on other sites More sharing options...
adrian Posted November 25, 2020 Share Posted November 25, 2020 I just took a look at your code and saw that you're already using preg_match_all so I could actually just add: \[\[([\w]+)\]\] as a non translated string and that takes care of excluding Hanna codes, so I think we are all good actually, although it might be nice to note this in the settings. 1 Link to comment Share on other sites More sharing options...
adrian Posted November 26, 2020 Share Posted November 26, 2020 @FireWire - is this the recommended way to use directly via the API? A bit off topic, but do you know why the returned text ends with a period? It also happens with some other phrases and is consistent with the results returned on the site here: https://www.deepl.com/en/translator but it's weird to me that it tries to fix grammar in some cases but not others - maybe it assumes that this is a complete sentence, but things like "the dog" are not and so in that case it doesn't add the period. Do you know if there is a setting for this? Link to comment Share on other sites More sharing options...
FireWire Posted November 26, 2020 Author Share Posted November 26, 2020 1 hour ago, adrian said: @FireWire - is this the recommended way to use directly via the API? A bit off topic, but do you know why the returned text ends with a period? It also happens with some other phrases and is consistent with the results returned on the site here: https://www.deepl.com/en/translator but it's weird to me that it tries to fix grammar in some cases but not others - maybe it assumes that this is a complete sentence, but things like "the dog" are not and so in that case it doesn't add the period. Do you know if there is a setting for this? Yep. That's a proper syntax to make a call. The text string for translating can also be an array of strings (either/or) which will be returned in the same order as you submitted, good for bulk translation. The DeepL class is and will be wrapper for the DeepL API so it should be as predictable as using it directly. The added period is not something I've seen during my usage so unfortunately I don't have experience. We've been translating one and two word strings, such as text for buttons, and they've never come back with any additional punctuation. The API has additional parameters which seem to be much more granular but I wanted to see how basic translation works out for PW implementation and then add additional features as necessary rather than fully integrating the API with things that would go unused. Also, since you're calling the API directly, you can wrap text to be ignored like this: <fluency-ignore>Don't translate me!</fluency-ignore> That's what's used in the DeepL class to ignore strings configured in PW. If you add them yourself the DeepL class will remove them automatically after translation. 1 Link to comment Share on other sites More sharing options...
FireWire Posted November 26, 2020 Author Share Posted November 26, 2020 @adrian I just pushed an update to the dev branch. It adds a 4th parameter to the translate() method in both Fluency and DeepL classes. You can pass a key/value with any additional DeepL API parameters to experiment with. There's a DeepL API parameter called preserve_formatting, and by default it is off- so it will try to correct punctuation. Try the dev branch and update your call with this then pass it the string that was giving you problems and see if it helps: That should tell DeepL to stop trying to correct you. Let me know if that does anything for you. 1 Link to comment Share on other sites More sharing options...
adrian Posted November 26, 2020 Share Posted November 26, 2020 Thanks @FireWire - just testing the dev version and a couple of things: 1) Parse Error: syntax error, unexpected '$output' (T_VARIABLE) (line 230 of site/modules/Fluency/classes/DeepL.class.php) I added the missing semi-colon to fix that. 2) Because the new $configs parameter is actually the 5th parameter for the translate method, you need to specify something for the 4th one ($ignoredStrings). So what worked for me, was this: which is great because now there is no period at the end, and also they didn't capitalize the first letter. I do wonder if there is much advantage for API use to use your class over a direct call, but it seems like there might be some added value, like the automatic stripping of any <fluency-ignore> tags etc. Anyway, thanks for the update - that's great and I'll be sure to look into the API to see what other options might be worth playing with. Link to comment Share on other sites More sharing options...
adrian Posted November 26, 2020 Share Posted November 26, 2020 Just wondering if the translate method in the Fluency class should be public so we can call the translate method more easily. I made that change locally, and now I can do: As you can see, now I can load the module and call translate directly, without the need to get and apply the API key. I guess this has more overhead but not really sure if it is significant. What do you think? 1 Link to comment Share on other sites More sharing options...
FireWire Posted November 26, 2020 Author Share Posted November 26, 2020 10 hours ago, adrian said: Thanks @FireWire - just testing the dev version and a couple of things: 1) Parse Error: syntax error, unexpected '$output' (T_VARIABLE) (line 230 of site/modules/Fluency/classes/DeepL.class.php) I added the missing semi-colon to fix that. 2) Because the new $configs parameter is actually the 5th parameter for the translate method, you need to specify something for the 4th one ($ignoredStrings). So what worked for me, was this: which is great because now there is no period at the end, and also they didn't capitalize the first letter. I do wonder if there is much advantage for API use to use your class over a direct call, but it seems like there might be some added value, like the automatic stripping of any <fluency-ignore> tags etc. Anyway, thanks for the update - that's great and I'll be sure to look into the API to see what other options might be worth playing with. I fixed that missing semicolon and made the translate method public and pushed it to dev- excellent feedback. Apologies for the missed semicolon and wrong parameter explanation. I was coding and pushing without testing since I wasn't able to do that at the moment. My goal is to make the Fluency->translate() method a more refined way to call the DeepL API. For instance, if you use the Fluency->translate() method, it will always account for the settings made in the module's PW config screen whereas a direct DeepL->translate() call won't. There could be advantages to using either depending on your use case, but the methods will also always take the same arguments so that either can be used interchangeably and predictably. And- as you noted, calling the Fluency->translate() method requires no manual API key insertion. I personally don't like having to stuff empty arguments into a method call, like how you had to provide an empty "[]" array before adding the config argument. The ignored strings array may be something I merge into the config parameter. It makes sense to me since if you're calling the Fluency->translate() method and have ignored strings configured then you might be passing an empty array more often than not, or the DeepL->translate() method and do not require ignored strings. This may be a breaking change coming up, so I'll post back here when that gets pushed to master. I want to do some more research on the preserve_formatting parameter and consider if it should be set to true by default. Since we're working with CMS input there should be a level of expectation that the translation service does not try to correct the user but rather deliver a more direct translation which would prevent the situation you saw. Overall, if there's anything set by default in the module I want to add a config screen toggle for it so that the developer can make the ultimate choice and be able to control how the module works. 1 Link to comment Share on other sites More sharing options...
adrian Posted November 26, 2020 Share Posted November 26, 2020 Thanks @FireWire - all that sounds great! Link to comment Share on other sites More sharing options...
FireWire Posted November 26, 2020 Author Share Posted November 26, 2020 Heck. I'm just going to modify the module so that all (or as many as possible) API parameters can be accessed/configured in the module. The developer should have as much control as possible. 1 Link to comment Share on other sites More sharing options...
adrian Posted November 26, 2020 Share Posted November 26, 2020 Thanks! As and FYI and a question to any Spanish / French / Italian (and any other languages that have gender specific nouns / verbs), I submitted a support ticket to DeepL yesterday asking about a way to specify the gender of names because of this scenario. my friend John -> mi amigo John my friend Jenny -> mi amiga Jenny my friend Jody -> what do we do here - DeepL doesn't know if Jody is male or female because it is used as both. I proposed to them that it might be nice to be able to send something like this to be translated: my friend <male>Jody</male> Of course this is a very simple example of gender translation issues (there are verb conjugation / contraction issues like ayudarle / ayudarla), but you get the idea. Actually, I barely know what I am talking about with this stuff but the process of investigating is helping my very limited spanish. DeepL is actually very impressive with this stuff. Anyone out there with any more experience have any thoughts? 1 Link to comment Share on other sites More sharing options...
xportde Posted November 27, 2020 Share Posted November 27, 2020 @FireWire Thank you for the page-edit-fix, this was one of the most important things to make this module usable in real! Quote Out of curiosity- do you see a need to break down access to the translator tool to a specific permission? I'm curious about that idea but didn't think there was a use case. From our point of view, a separate translation permission would make perfect sense, because not everyone, who can edit a page, should do translations. Mainly because the service is subject to a charge, one should be sensitive when giving the translation service permission. Best regards, Thomas. 1 Link to comment Share on other sites More sharing options...
FireWire Posted November 27, 2020 Author Share Posted November 27, 2020 13 hours ago, xportde said: @FireWire Thank you for the page-edit-fix, this was one of the most important things to make this module usable in real! From our point of view, a separate translation permission would make perfect sense, because not everyone, who can edit a page, should do translations. Mainly because the service is subject to a charge, one should be sensitive when giving the translation service permission. Best regards, Thomas. Ah yes, that is a very good point given that there is a cost factor. I will add a dedicated permission in the next versioned release. Thank you for the feedback! Link to comment Share on other sites More sharing options...
adrian Posted December 1, 2020 Share Posted December 1, 2020 @FireWire - not sure if this is a bug with your module, or the DeepL API, but if I include a two word business name, eg: "Amazon Prime" in the Global non-translated strings, and then I translate a sentence that is like this: Visit Amazon Prime. Next Sentence. then the returned translation looks like this: Visite Amazon PrimePróxima frase. with the period and space removed. Please let me know if you have any problems reproducing. I have discovered that I actually don't need to add it to the non-translated strings because DeepL is smart enough to realize that uppercase in the middle of a sentence shouldn't be translated, but I expect this might be an issue with lower case non-translated strings. Thanks for looking into it. Link to comment Share on other sites More sharing options...
adrian Posted December 1, 2020 Share Posted December 1, 2020 For some reason I can no longer call the API. I am seeing errors like this: "HTTP Code: 0 API Message: No message returned Request Endpoint: /translate Parameters:" On pages that call the API, I am also seeing this: "Service Unavailable The server is temporarily unable to service your request due to maintenance downtime or capacity problems. Please try again later." I am not sure exactly what is needed yet, but it seems like there might need to be a try/catch or some other approach so that things just don't break. I am also going to contact DeepL and see if there is a known issue at the moment and how often things like are likely to happen, because it could be really messy for my use case using the API directly. I guess for translating fields in the admin it's not such a big deal, although you still want to make sure it's handled properly. Link to comment Share on other sites More sharing options...
adrian Posted December 1, 2020 Share Posted December 1, 2020 FYI - looks like everything is back up again, but I'll still be sure to post here whatever I hear back from DeepL as to what the problem was. Link to comment Share on other sites More sharing options...
adrian Posted December 1, 2020 Share Posted December 1, 2020 Ok, so here is the reply from DeepL "We experienced a temporary technical problem earlier today. As you noticed, the issue has already been resolved. It is rather seldom that we experience outrages. Should this occur, we strive to resolve the issue as fast as possible, as you have seen it today." Nothing terribly illuminating, but it does suggest that outages are to be expected (as I suppose they are with any service), but I think this means that we do need a friendlier way of catching and handling errors. 1 Link to comment Share on other sites More sharing options...
FireWire Posted December 1, 2020 Author Share Posted December 1, 2020 4 hours ago, adrian said: Ok, so here is the reply from DeepL "We experienced a temporary technical problem earlier today. As you noticed, the issue has already been resolved. It is rather seldom that we experience outrages. Should this occur, we strive to resolve the issue as fast as possible, as you have seen it today." Nothing terribly illuminating, but it does suggest that outages are to be expected (as I suppose they are with any service), but I think this means that we do need a friendlier way of catching and handling errors. Proper error handling is definitely on the implementation list. I guess it just got upgraded to a higher priority item now that we've seen that. For what it's worth I've been playing around with the API for months and haven't experienced that. Not downplaying the importance of error handling, just hoping that means they're correct when they say that it happens seldomly. I'll be pushing more work when I can get some time dedicated to the module. I'm slammed at work right now but I will prioritize error handling when I'm back on the project. Thank you for reporting this here as someone might look for answers if this happens again, even with error handling. 2 Link to comment Share on other sites More sharing options...
adrian Posted December 7, 2020 Share Posted December 7, 2020 @FireWire - just a reminder to remove the bd($content) call in Fluency.module.php Also, just want to agree with @xportde's request for a dedicated permission. I have already changed it locally to add a "fluency" permission. In my use case I actually needed to give the guest user this permission so that I could do translations via a cron called script. Thanks again for your work on this - I am now using it in production. 1 Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now