[Translator] [Xliff] Add support for target attributes. #15452
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| */ | ||
| private function hasMetadataArrayInfo($key, $metadata = null) | ||
| { | ||
| return |
There was a problem hiding this comment.
this part is hard to read and understand IMO
There was a problem hiding this comment.
it should be on one line.
|
Thanks for your work! 👍 ping @aitboudad |
| * | ||
| * @return array | ||
| */ | ||
| private function setNotesMetadata($noteElement = null, $encoding = null) |
There was a problem hiding this comment.
the function name is weird. It is not setting anything. It is reading attributes.
And the $noteElement argument should be typehinted
There was a problem hiding this comment.
What name do you suggest? What about parseNotesMetadata or getNotesMetadata ?
There was a problem hiding this comment.
parseNotesMetadata looks good to me.
The previous implementation ignored attributes in target nodes in xliff files so they were lost when you load and then dump the same file. This change should fix that problem.
7d97c1b to
b913695
Compare
|
Comments addressed. |
|
👍 Status: Reviewed |
|
👍 |
|
Thank you @marcosdsanchez. |
aitboudad
added a commit
that referenced
this pull request
Aug 14, 2015
…. (marcosdsanchez) This PR was merged into the 2.8 branch. Discussion ---------- [Translator] [Xliff] Add support for target attributes. The previous implementation ignored attributes in target nodes in xliff files so they were lost when you loaded and then dumped the same file. This change should fix that problem. | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A Commits ------- b913695 Add support for target attributes.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous implementation ignored attributes in target nodes in xliff files
so they were lost when you loaded and then dumped the same file. This change
should fix that problem.