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

loctool, ilib-po: Add support to the loctool for saving intermediate files in po format #31

Merged
merged 24 commits into from
Dec 19, 2024

Conversation

ehoogerbeets
Copy link
Contributor

@ehoogerbeets ehoogerbeets commented Nov 27, 2024

  • ilib-po updates
    • Added the ability to encode ResourceArray resources in a po file in a reversible way
    • Added support for encoding unique keys for resources that are different than the English string
    • Added support for datatypes and project names for each resource and as the default for the whole file
    • Fixed a bug where you could not have resources with non-json format comments on them. Now it only parses the json format if the comment starts with '{'
  • loctool updates
    • Added command-line switch --intermediateFormat to switch between xliff or po intermediate files
    • Added IntermediateFile class with a factory method getIntermediateFile() that returns either a PO or Xliff intermediate file. The factory method can also determine the intermediate file type from the file name extension (.po or .xliff) of the requested file.
    • Use the new intermediate file support to save the extracted files and the new strings files in either format
    • Use the new intermediate file support to read in translation files in either format

@ehoogerbeets ehoogerbeets self-assigned this Nov 27, 2024
@ehoogerbeets ehoogerbeets force-pushed the loctool-add-po-support branch from 145560e to 4d4ab4c Compare December 5, 2024 02:16
@ehoogerbeets ehoogerbeets changed the title Add support to the loctool for saving intermediate files in po format loctool, ilib-po: Add support to the loctool for saving intermediate files in po format Dec 5, 2024
@ehoogerbeets ehoogerbeets force-pushed the loctool-add-po-support branch from f0a2ad0 to 36d7c09 Compare December 5, 2024 18:36
@ehoogerbeets ehoogerbeets marked this pull request as ready for review December 7, 2024 01:42
@ehoogerbeets ehoogerbeets force-pushed the loctool-add-po-support branch 2 times, most recently from e8ddbab to 022fa23 Compare December 11, 2024 06:26
Copy link

changeset-bot bot commented Dec 11, 2024

🦋 Changeset detected

Latest commit: b79d491

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
ilib-po Minor
loctool Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the formatting-only change intentional?

Comment on lines 186 to 191
defaultDataType: string | undefined = this.datatype,
datatype: string | undefined,
defaultProject: string | undefined = this.projectName,
project: string | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove default* local variables and use the instance-level value directly whenever you intend to fall back to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't do that -- these ones have a purpose that is different than the instance-level values. The default data type and project name can be defined in the headers of the po file and can be different than the ones specified parameters to the POFile constructor. We still need a place to store that. The default data type and project are parsed out of the header of the po file on lines 204-211. If they are not there in the file, they fall back to the values for the instance. Maybe I should call them defaultFileDataType and defaultFileProject instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I missed that. So basically, the value attached to the produced resource instance fallback should be 1. from unit 2. from file header 3. from parser instance right? In that case, maybe use the word file instead default (rather than both)

Comment on lines +726 to +727
var fileFormat = this.settings.intermediateFormat || "xliff";
var extractedPath = path.join(dir, base + "-extracted." + fileFormat);
Copy link
Contributor

Choose a reason for hiding this comment

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

Extension string should be separate from the format identifier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now, they are exactly the same, so it is a little silly to introduce a mapping right now.


/**
* The name of the project that the resources belong to
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document the default value

@ehoogerbeets ehoogerbeets force-pushed the loctool-add-po-support branch from 4fca28f to d056255 Compare December 18, 2024 23:27
... where the key is different than the source string
- includes a factory method to create a new intermediate file
- in the po file format generator, if the comments don't have the
  path, use the resource path to create the colon comment
- added @jest/types so that the IDE stops complaining
- it is usually noted in the header. If it is not there, default
to English
- both reading and writing with both PO and xliff files
- getIntermediateFile went into its own factory file
- the subclasses of IntermediateFile get their own file
- IntermediateFile.js is now just a simple abstract class
- controlled by the command-line argument
- each resource can have its own datatype represented as a #d comment
- each file can have a default datatype given in the "Data-Type:" header
- each resource can have its own project name represented as a #p comment
- each file can have a default project name given in the "Project:" header
- based on the intermediateFormat settings
- pass the data type and project to the generator
- all unit tests pass
- add src to the files in the package
- workspace:* dependencies mean that when you create a tgz file for
  testing, then you have to make a tgz file for a whole tree of
  as-yet unpublished and unrelated changes in other parts of the
  monorepo in order to satisfy all the dependencies. This is onerous
  for a simple little test. The solution is to use dependencies
  like workspace:^1.11.0 when you know you only need the older
  1.11.0 of the other package for testing.
- moved release notes into the changesets
- change dependencies into workspace:^
@ehoogerbeets ehoogerbeets force-pushed the loctool-add-po-support branch from 7eee01d to b79d491 Compare December 19, 2024 18:32
@ehoogerbeets ehoogerbeets merged commit 0672f4c into main Dec 19, 2024
1 check passed
@ehoogerbeets ehoogerbeets deleted the loctool-add-po-support branch December 19, 2024 18:55
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