Skip to content

Conversation

@renovate
Copy link
Contributor

@renovate renovate bot commented Oct 6, 2025

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
gettext_i18n_rails_js "~>1.3.0" -> "~>1.4.0" age adoption passing confidence

Release Notes

webhippie/gettext_i18n_rails_js (gettext_i18n_rails_js)

v1.4.0

Compare Source

  • Added support for doing conversion for different domains and rails engines (@​adamruzicka)

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR was generated by Mend Renovate. View the repository job log.

@renovate renovate bot requested a review from Fryguy as a code owner October 6, 2025 19:12
@renovate renovate bot added the dependencies label Oct 6, 2025
@renovate renovate bot assigned Fryguy Oct 6, 2025
@Fryguy Fryguy assigned jrafanie and unassigned Fryguy Oct 17, 2025
@Fryguy
Copy link
Member

Fryguy commented Oct 17, 2025

@kbrock @jrafanie Did we ever figure out the quotes issue here? Did webhippie/gettext_i18n_rails_js#55 break us?

@jrafanie
Copy link
Member

yeah, it broke us... I haven't come back to it yet

@kbrock
Copy link
Member

kbrock commented Oct 17, 2025

I'm pretty sure I fixed it.
But then that other change went in.
Maybe we ended up double doing that stuff

I haven't revisited either

@Fryguy
Copy link
Member

Fryguy commented Oct 17, 2025

Which other change? If you mean the Po to Json change, that only kicks in after we create the Po and merge it. The changes in the latest version break the po generation itself.

@kbrock
Copy link
Member

kbrock commented Oct 18, 2025

Do we have a simple (bash?) reproducer here?

@jrafanie You want to pair Monday?

@Fryguy
Copy link
Member

Fryguy commented Oct 20, 2025

@kbrock You can just run rake locale:update_all with this PR

@renovate renovate bot force-pushed the renovate/gettext_i18n_rails_js-1.x branch from 5b22100 to 81275e7 Compare October 20, 2025 13:32
@jrafanie
Copy link
Member

@kbrock You can just run rake locale:update_all with this PR

you'll need a i18n database to run that:

i18n:
  <<: *base
  database: vmdb_i18n

@Fryguy
Copy link
Member

Fryguy commented Oct 20, 2025

I was trying to build something isolated, but I think gettext_i18n_rails_js needs an entire rails project

@Fryguy
Copy link
Member

Fryguy commented Oct 20, 2025

BTW, note that the latest version is actually 2.2.2 - not sure why we are still on 1.x

@kbrock
Copy link
Member

kbrock commented Oct 21, 2025

ok, I created an isolated rails project.

TL;DR: gettext is in charge of creation of the pot and it does it correctly.

  • I'm not seeing the error we are seeing in miq.
  • So we introduced the issue.
  • This issue is no where near the _js gem. This is an error with pot generation.

So we probably introduce an issue via our code...

  • We monkey patched/broke the ruby extraction code (i.e. extract from _() in ruby files)
  • We introduce an issue with the code that concatenates yaml and ruby extraction.

Details

gem version miq latest notes
ruby 3.4.7 3.1.?
rails 8.0.3 7.1.*
fast_gettext 3.1.0 4.1.0 tried with and without this gem
gettext 3.5.1
gettext_i18n_rails 2.0
gettext_i18n_rails_js 2.2.2 1.3.1

I scaffold'd users, modified users/index.html.erb and ran rake gettext:find.

I did a simple rake gettext:find which is essentially calls rake gettext:po:update

html.erb convert quotes `locale/app.pot
_("All1 'Users'")) "All1 'Users'" "All1 'Users'"
_("All2 \'Users\'") "All2 'Users'" "All2 \\'Users\\'"
_("All3 \"Users\"") "All3 \"Users\"" "All3 \"Users\""
_('All4 "Users"') "All4 \"Users\"" "All4 \"Users\""
_('Show5 this \'user\'') "Show5 this 'user'" "Show5 this 'user'"
_('New6 \"user\"') "New6 \\\"user\\\"" "New6 \\\"user\\\""

Case 2 and 5 are unnecessary escaping. case 2 is what I expected the output to be, but technically incorrect for ruby. (unsure in the js, handlebars, and other cases) Not sure we should squabble over error cases.

@kbrock
Copy link
Member

kbrock commented Oct 23, 2025

Yea. That or that I added was buggy. Got a pr in there and hopefully this will all be fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants