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 i18n support to RStudio #1

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from
Draft

Add i18n support to RStudio #1

wants to merge 23 commits into from

Conversation

ca-scribner
Copy link
Owner

@ca-scribner ca-scribner commented Apr 14, 2021

WIP PR for adding i18n support to RStudio. Will fill this issue out with a better task list shortly.

i18n tasks:

  • Commands.cmd.xml:
    • Cmds
    • Menus
    • Shortcuts (blocker: how to handle shortcuts that depend on if statements?)
  • user-state-schema.json/user-prefs-schema.json
    • Update generate-prefs.R to emit java/gwt constants/properties files
    • Add optional enumReadable to user-prefs-schema.json to contain human readable "enum" text that should be translated (this is used as the display text for selection boxes in Global Options menus - eg: enum option quotes_and_brackets has enumReadable of Quotes & Brackets)
    • Add human readable enumerator texts to i18n (currently emitted as static strings in the .java definitions)
  • Global Options menus
    • (WIP) Options -> Code (EditingPreferencesPane.java)
      • Use text from user-prefs-schema.json to prevent duplicate text/translation effort (existing code has text hard coded here plus defined in the .json)
      • Checkboxes
      • Enumerators
      • Statically defined headings/other misc items TBD
      • Need to handle LineEndingsSelectWidget class. It implements the enumerator and human readable values directly, plus logic for whether to show all values or a subset. This class is also used by ProjectEditingPreferencesPane.java, which does not have the same access to preferences as EditingPreferencesPane.java. Need to investigate. Porting logic will be easy, but getting the translated values from the lineEndingConversion() PrefValue might be harder.
    • All other menus (Options -> *)
  • All other static strings in .java files
    • Basic proof of concept (using AboutDialog.java)
    • All other static strings not discussed above
      • First pass through strings, removing/annotating strings that are obviously ok (things like enumerator keys, etc)
      • i18n-ify all remaining true positives

Includes basic properties, but probably not a good workflow
Was this duplicate command intentional?  If yes, need to handle this in i18n properties/interface generator scripts
…perties

Really this makes more sense to be separate scripts or have a more modular entrypoint, but haven't done that yet.
- rearranges parsers and helpers to reduce duplication
- adds (untested) menu parser
- fix Command tests
…o other types

* Add lineage of element to Menu item names in output constants/properties file (eg: File->NewFile->R is now named "menu$File$NewFile$R") to resolve naming collisions
* Use lxml instead of ElementTree to support identifying parent from XML elements
* Add ElementParser.grep_from_element_tree() to encapsulate element search logic somewhere transferable (finding menu items is different from shortcuts, commands)
* Add i18n support for MenuEmitter
* Add MenuConstants.java/MenuConstants_en.properties (generated by xml_to_i18n.py)
* Refactor CommandConstants interface to CmdConstants to be consistent with xml tagging nomenclature.  Rename files generated using xml_to_i18n.py
Shortcut is working in progress.  Realized we need to also handle the if statements (refid alone is not enough - any combination of refid+if is a translatable string).  If statements have many possibilities (`if x.y.z.isMac() && isFirefox()`, etc) and can be generic so we don't know what'll be added later.  These are typically outlined in `org.rstudio.core.client.BrowseCap` and `org.rstudio.studio.client.application.Desktop`.  Need to consider this more.

Might be able to just concatenate refid+if then convert to only valid class name characters.  Is that workable?  It'll make for terrible class names in the interface/properties file, although maybe we could annotate them with better data in the comments
…cription

Updated generate-prefs.R to emit i18n constants and properties files and to use those in the emitted Prefs/State accessor classes for title and description.

Code appears to be working, but I cannot find where these titles/descriptions are actually used.  I thought they'd be in the Global Options menu, but those menus are described by static strings in the menu definitions.  I cannot find anywhere else these titles/descriptions are used for.
…mbers)

* Adds signatures to numericPref and checkboxPref that parse the preference Title and Description and pass them to the element in the Pane.  Some text in the EditingPreferencesPane has been ported to the user-prefs-schema.json because it is more descriptive
* Above signatures add tool tips to any option that has a description in the users-prefs-schema.json file.  This adds tooltips to many options that previously did not have them.

Most Titles/Descriptions are the same or very similar to the static strings that were in EditingPreferencesPane before, but some have slight changes.  Need to check whether there's any material reversions that have occurred.

Enumerators and statically defined headings are still todo
@ca-scribner
Copy link
Owner Author

Intermediate update:

I plan to clean it up and make it more approachable, but for now the basics that have been done are:

  • modify src/gwt/src/org/rstudio/studio/RStudio.gwt.xml and RStudioSuperDevMode.gwt.xml to enable GWT i18n, passing of locales, etc.
  • directly change AboutDialog.java to use i18n constants (this was an early spot where I just wanted to make sure things were working. The translations there were just made to be easy to spot, not real translations ;) )
  • for Commands.cmd.xml:
    • Add a parser (/src/gwt/tools/i18n-helpers/*) that reads the Commands.cmd.xml file and generates *Constants.java and *Constants_en.properties for all Cmds and Menus. Shortcuts need to be handled here too, but I'm a little stuck on them atm because shortcuts have more than just a name (shortcutA might have different values based on generic if statements that are written in code. Not sure how to handle that in a serialized java constant name...)
    • Modify CommandBundleGenerator.java, MenuEmitter.java, etc., to use the new java interfaces from previous step
    • Naming for menu java constants got a little messy. I had name collisions (an item in File might have the same name as an item in Edit), so I used fully qualified names. This means New _File in the file menu is named main$_File$New__FileLabel (nested menus indicated by '$', spaces become '', and there's an existing '' for the shortcut key). It feels verbose and the $ looks awful, but it is readable and a valid java name... open to suggestions
    • Sorry that I've added a bunch of .py files to RStudio. Shamefully, modifying generate-prefs.R below was the first coding I've ever done in R, so writing this up in python was a lot easier
  • For user-prefs-schema.json and Global Options menus
    • update generate-prefs.R to emit Constants and Properties (src/gwt/src/org/rstudio/studio/client/workbench/prefs/model/UserPrefsAccessorConstants.java, etc) files for all the user settings. Modify emitted UserPrefsAccessor.java to use those. This is WIP and does not yet fully handle enumerations and probably some other things, but is largely complete. These emitted Constants/Properties can then be consumed to build the Global Options menus and also referenced by the command pallette, etc.
    • First attempts here have been using the EditingPreferencesPane.java. It is now mostly using i18n constants driven from the user-prefs-schema.json file rather than hard coded into the .java file. I noticed that the text in EditingPreferencesPane.java was nearly identical to the text in user-prefs-schema.json, so for now I'm using the .json text and reconciling any minor differences.
    • Added some constructors/factories to remove some boilerplate code from EditingPreferencesPane.java. Mostly this is just adding a factory for each type that can read a preference directly (eg: since the text labels written in EditingPreferencesPane.java were virtually identical to the titles in the .json file, I've changed calls like checkboxPref("Some text label", prefs.somePreference()) to checkboxPref(prefs.somePreference()) where we infer the checkbox's text directly from prefs.somePrefernece()

The new i18n for Commands.cmd.xml would follow a similar workflow to that done with preferences/generate-prefs.R. Modify the xml (or json), rerun the xml->i18n script (or rerun generate-prefs.R), commit the generated files and use them in java to wire in the new options/commands, etc. In all cases, the default text is matched to the xml file and an English properties file can be generated to start. For practical purposes it might be smart to add more context to the properties files in future.

Outstanding items/questions:

  • Is there a reason why I shouldn't reconcile the differences between Global Options panes and user-prefs-schema.json? I think they're usually quite trivial, and it means for less duplication
  • There are a lot of .java files with static strings (EditingPreferencesPane.java has plenty that describe subheadings, for example). I've only proved the concept to make sure it worked as expected (see About Dialog), but I'll have to go through and fish these all out. There's going to be a lot of new *Constants.java files. I think it makes sense to have a 1:1 match between files (eg: ExistingFile.java uses ExistingFileConstant.java for constants), but I'm open to other requests
  • Still trying to find any other non-standard uses of translatable material.

* Add human readable text corresponding to enumerators in user-prefs-schema.json.  Add handling of these in generate-prefs.R to export human readable enumerators to translation files/use those translations instead of hard coded text in EditingPreferencesPane.java.
* Add convenience signatures to SelectWidget
* Convert EditingPreferencesPane.java's SelectWidget's to i18n
* Remove CommandConstants.properties file that should have been removed previously
* Minor updates to user-prefs-schema.json to reconcile differences with hard-coded strings in EditingPreferencesPane.java
Adds PreferencesPaneConstants properties, interfaces
ca-scribner and others added 3 commits May 11, 2021 16:02
CRANMirror implements a .getDisplay() method for showing a human readable representation of itself.  PackagesPreferencesPane.java and ChooseMirrorDialog.java both use this, but only when CRANMirror.host != "Custom", instead showing CRANMirror.url in this case:
* These two references are the only usages of getDisplay(), and are direct copies of each other.  Both apply the same logic in whether to use getDisplay() or create their own displayed text, and when creating their own displayed text they produce the same result (using the URL as the display value
* Determining whether a CRANMirror is "Custom" depends on the somewhat brittle fact that .host=="Custom" when a mirror is defined by URL alone.  This is hard for the CRANMirror maintain if it was ever updated (need to find hard coded strings of "Custom" in the codebase and update them)

Proposed changes here address the above by centralizing the display logic of PackagesPreferencesPane.java and ChooseMirrorDialog.java in CRANMirror.  The following added/changed methods are provided in CRANMirror:
* .getCustomEnumValue(): Serves as an enumerator to specify the string label that denotes a custom object.  This would be more cleanly implemented as an enumerator string, but CRANMirror extends JavaScriptObject which cannot have class-level attributes
* .getSecondaryEnumValue(): Similar to Custom, but implements another enumerator for a "Secondary" mirror
* .setAsCustom(): This sets the object as a "Custom" (url-only) object, replacing the previous pattern where the user of the object would set .host="Custom" and .name="Custom" themselves
* .isCustom(): API for public to assess whether a CRANMirror is or is not "Custom".  Implemented by testing `host==getCustomEnumValue()` (eg: host=="Custom"), which is how consumers of this object previously performed the test themselves
* .getDisplay(): Implements the logic used by PackagesPreferencesPane.java and ChooseMirrorDialog.java, where the display text will be the URL if this is a custom CRANMirror.
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.

2 participants