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

More space savings #7181

Merged
merged 32 commits into from
Dec 1, 2022
Merged

More space savings #7181

merged 32 commits into from
Dec 1, 2022

Conversation

jepler
Copy link
Member

@jepler jepler commented Nov 8, 2022

Two big ones:

  • eliminating a bunch of incorrect, never-used QSTRs
  • Fixing a problem where the French translation was using Unicode code points above 255 again; this almost certainly pops French out of being the largest translation again

We should add a check for langauges that we expect all the chars to fit in uint8_t actually do, otherwise firmware grows by up to 500+ bytes...

@jepler jepler marked this pull request as draft November 8, 2022 22:42
@jepler
Copy link
Member Author

jepler commented Nov 8, 2022

Marking this as a draft, we can pick and choose from these improvements when we have a need for space ...

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

@Neradoc what do you think of the French apostrophe change? I'm not sure we could get the regular translators to do that, or if it looks wrong.

shared-bindings/busio/I2C.c Outdated Show resolved Hide resolved
@Neradoc
Copy link

Neradoc commented Nov 9, 2022

@Neradoc what do you think of the French apostrophe change? I'm not sure we could get the regular translators to do that, or if it looks wrong.

As far as I know it's only ever auto inserted by software, like Word, a CMS and such. Nobody is gonna notice the difference. It's also not even used consistently in CP.

@jepler
Copy link
Member Author

jepler commented Nov 9, 2022

I also had to respell an œ as oe, and made changes in a few other translations as well.

we could look into a different way of storing the data (such as utf-8) but last time I looked there was a real inconvenience to storing values[] in a way that can't simply be indexed

I also noticed that some other languages, like ru, use just a small number of codes well outside the 0-255 range. A much less complicated rule could remap the 160..255 values to those code points and potentially save space. In the board I've been using as my benchmark (adafruit_proxlight_trinkey_m0), with all the uint8 fixes, the fullest languages are ru and el (greek) which could benefit from this treatment.

@jepler jepler marked this pull request as ready for review November 9, 2022 17:33
@dhalbert
Copy link
Collaborator

@jepler Did you back out all the French spelling changes? That was the thing I was worried about. If so, I'll approve.

@dhalbert
Copy link
Collaborator

Needs a re-merge from upstream.

MP_REGISTER_MODULE would use identifiers like
"MODULE_DEF_MP_QSTR___FUTURE__" which would in turn cause
a QSTR to be generated for it. This wasn't desirable, because the
qstr would never be used.

This clears out quite a bit of flash storage on the proxlight trinkey.
These are turned into TRANSLATE() messages now, so the qstr version
would not be used.
@jepler
Copy link
Member Author

jepler commented Nov 30, 2022

I'll rebase.

The french translation leaves œ but still changes a smart-quote to a regular quote.

@jepler
Copy link
Member Author

jepler commented Dec 1, 2022

@dhalbert ready for hopefully one last review

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Looks good! More bytes to use up :)

@dhalbert dhalbert merged commit b41f746 into adafruit:main Dec 1, 2022
@dhalbert
Copy link
Collaborator

dhalbert commented Dec 1, 2022

After merging this, I got an email from weblate:

Could not merge the repository.

Weblate could not merge upstream changes while updating the repository.

Auto-merging locale/nl.po
Auto-merging locale/fr.po
Auto-merging locale/fil.po
CONFLICT (content): Merge conflict in locale/fil.po
Automatic merge failed; fix conflicts and then commit the result.
 (1)

Typical workflow for fixing merge conflicts

  1. Commit all pending changes in Weblate and lock the translation component.
    wlc commit; wlc lock
  2. Add Weblate exported repository as a remote.
    git remote add weblate https://hosted.weblate.org/git/circuitpython/main/ ; git remote update weblate
  3. Merge Weblate changes and resolve any conflicts.
    git merge weblate/main
  4. Push changes into upstream repository.
    git push origin main
  5. Weblate should now be able to see updated repository and you can unlock it.
    wlc pull ; wlc unlock

Check the FAQ for info on how to resolve this.

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.

None yet

3 participants