-
Notifications
You must be signed in to change notification settings - Fork 18
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
gettext or replacement not working in cljs #35
Comments
Hi, I finally started my project using your library ! But my project is in french, I would love using your predefined verbs but i'm not sure how I should replace your english dictionary with mine translated. |
Hi, nice to see your interest on the library. I have rewritten big chunks of the library since I first implemented the gettext integration, so I'm not really sure how much is it working right now. I think it's not very hard without some hacking, but you'll have to dig through the code and test a bit. Are you targetting the terminal (clojure) or the browser (clojurescript) or both? Here you can see how the translation dictionaries are defined: https://github.com/facundoolano/advenjure/blob/master/src/advenjure/text/ And here is stab I did at porting gettext to work with the newer versions of the library (specifically to make it compatible with clojurescript): https://github.com/facundoolano/house-taken-over/blob/master/src/advenjure/text/en_past.cljc.hack I honestly don't remember if that one worked or not, but you could start by trying that in your game. |
If that doesn't work another option is to clone this repo, and update the default dictionary pointed in this line: https://github.com/facundoolano/advenjure/blob/master/src/advenjure/gettext/core.cljc#L7 |
Hi ! Indeed, I target mainly js. But hey, if you can have the jvm for free I won't say no.
It works for the JVM, but did not for clojurescript (there is no Maybe (that's just a proposition) we could use something like an atom for setting the dictionary instead of a dynamic var since it is easy to change the value of an atom. I think default dictionary (every phrase strings present in the API except for verbs maybe since they can be redefined by verb-map) should use keywords instead of full text since full text is more error prone when defining a new dictionary. If you're OK I think I can do a PR for that too. |
yeah the atom is not a problem. I originally used a dynamic binding because gettext was extracted as its own library, and it was only meant to be used in clojure (clojurescript), so I went with the most clean/idiomatic approach. But at this point I don't care to use the gettext library, my main priority is to be able to modify the language dict in both targets without too much effort, so if it accomplishes that, your PR is more than welcome. Regarding kws instead of strings I don't know if I'm convinced, I like the fact that you can see the text inline in the code, and use that text by default if the dictionary does not override it. |
I propose the PR #41 . I see your point for kws. Indeed having the string as is in the code is quite comfortable. The fact is it's not easy for a user like me to redefine your default dictionary. REPL workflow helps a lot since I can inspect the dictionary and copy-paste it to my code then modify it. So it's not that unbearable, just a bit confusing at first, maybe it should just be documented somewhere on the wiki or the README. |
Yeah definitely we're short on documentation (it's planned here #19). |
@CharlesHD merged your second solution. Also created #44 to fix what you pointed about the keys being outdated. If you end up making your translations to french, please send a PR with the dictionary. |
Cool ! Wasn't here this weekend, glad it helped. I'll PR you a neutral french dictionary asap. |
I'm reopening this, since my fix seems to have broken the cljs version |
I don't think we can hack our way out of this without rethinking the problem, the whole gettext thing was structured around the idea of pointing to a var from configuration and requiring it dynamically, but CljoureScript can't really handle arbitrary requires in the middle of the code, so I don't see this happening (it doesn't even have require in the version we were using). I will revert recent changes and keep this open to come up with a different solution. |
No description provided.
The text was updated successfully, but these errors were encountered: