hedyorg / hedy

Hedy is a gradual programming language to teach children programming. Gradual languages use different language levels, where each level adds new concepts and syntactic complexity. At the end of the Hedy level sequence, kids master a subset of syntactically valid Python.
https://www.hedy.org
European Union Public License 1.2
1.29k stars 284 forks source link

[WEBLATE] Install plugin for line length #4501

Open Felienne opened 11 months ago

Felienne commented 11 months ago

So I have the answer as to why this keeps happening: the way Weblate and gettext calculate the length of the string is fundamentally different, one uses byte lenght and the other character length, there is an issue for that on the Weblate repository that is marked as wontfix, so no matter what width we choose, there will always be a difference in the way the lines are wrapped.

        So I have the answer as to why this keeps happening: the way Weblate and gettext calculate the length of the string is fundamentally different, one uses byte lenght and the other character length, there is an [issue for that](https://github.com/WeblateOrg/weblate/issues/6350) on the Weblate repository that is marked as wontfix, so no matter what width we choose, there will always be a difference in the way the lines are wrapped. 

The way I see we can fix this is by installing a plug-in to Weblate and set the option to not wrap lines (apparently someone had problems with that too, but I'm not sure if they messed up something or if its actually an issue, still they marked it as wontfix), but as it concerns to this particular PR I don't think there's a way to prevent the changes done to the line spacing, at least in the state that main is right now, perhaps after applying the plug-in and if the lines aren't wrapped I can apply the same option here and there won't be all of those differences.

What would you like to do? Can I install the plug-in to Weblate or are there other considerations?

_Originally posted by @jpelay in https://github.com/hedyorg/hedy/pull/4492#discussion_r1327250459_

Felienne commented 11 months ago

Hi @Mark-Giesen!

I am not sure this is easy to do, but since you are working on Weblate related things at the moment, is this something you could take a look at? (if you don't have time, I think we will close this since no one really has time to dive in and the issue is small)

Mark-Giesen commented 11 months ago

I am not sure this is easy to do, but since you are working on Weblate related things at the moment, is this something you could take a look at? (if you don't have time, I think we will close this since no one really has time to dive in and the issue is small)

Installing the add-on and setting it to --no-wrap is easy. I'm not sure if this is what is meant here en will solve the problem? Maybe some live discussing and testing would be an idea? Not the coming 2 weeks though.

Felienne commented 11 months ago

I am not sure this is easy to do, but since you are working on Weblate related things at the moment, is this something you could take a look at? (if you don't have time, I think we will close this since no one really has time to dive in and the issue is small)

Installing the add-on and setting it to --no-wrap is easy. I'm not sure if this is what is meant here en will solve the problem? Maybe some live discussing and testing would be an idea? Not the coming 2 weeks though.

Hi Mark! See the points of @jpelay above, Weblate now wraps the lines but babel wraps them differently and then we get changes all the time. If you can enable the plugin, we can see if that helps!

Mark-Giesen commented 11 months ago

Hi Mark! See the points of @jpelay above, Weblate now wraps the lines but babel wraps them differently and then we get changes all the time. If you can enable the plugin, we can see if that helps!

Add-On added and set to "wrap on newlines". This is the same behavior as gettext would do with the --no-wrap option. In the CONTRIBUTING.md file we tell users to use that option when updating with pybabel (which is a wrapper around gettext). This should now result in the same behavior on both sides.

Mark-Giesen commented 11 months ago

I could only find a reference to this --no-wrap option in CONTRIBUTING.md, this probably means we don't do an extract and update automaticly, only the compile step. I believe this was a conscious decision for build-time purposes, but maybe worth mentioning every now and then ;-)

Felienne commented 11 months ago

I could only find a reference to this --no-wrap option in CONTRIBUTING.md, this probably means we don't do an extract and update automaticly, only the compile step. I believe this was a conscious decision for build-time purposes, but maybe worth mentioning every now and then ;-)

Yeah we moved a lot of explanation to the wiki, see here: https://github.com/hedyorg/hedy/wiki/Hedy-Development-Process#adding-new-translation-keys

Should we add a no-wrap there?

Felienne commented 10 months ago

Hi @jtwaleson assigning this to you, so you can check in weblate. I think it is now all fine as it is, our add-on sets no wrapping:

image

(You can find the add-ons here: https://hosted.weblate.org/addons/hedy/adventures/ even when they are for the project, they always connect to a component)

But just to be sure, let's leave this issue open for now.

Mark-Giesen commented 10 months ago

Hi @jtwaleson assigning this to you, so you can check in weblate. I think it is now all fine as it is, our add-on sets no wrapping:

This issue doesn't seem to be assigned, but I think it's solved? By --no-wrap on both sides, only newlines will trigger a wrap, that must be equal on all sides, I assume? Only "issue" is we don't do --no-wrap anywhere automaticly, it's mentioned in the wiki for people that run pybabel by hand, but that might just be enough?

Felienne commented 10 months ago

Hi @jtwaleson assigning this to you, so you can check in weblate. I think it is now all fine as it is, our add-on sets no wrapping:

This issue doesn't seem to be assigned, but I think it's solved? By --no-wrap on both sides, only newlines will trigger a wrap, that must be equal on all sides, I assume? Only "issue" is we don't do --no-wrap anywhere automaticly, it's mentioned in the wiki for people that run pybabel by hand, but that might just be enough?

We weren't sure if it is fixed, therefore we left it open, if you think it can be closed, it probably can!

We could also add no-wrap to the on-build script? But I can't easily oversee whether that would help.

yilmazdurmaz commented 10 months ago

I might be missing the main idea here, so would be better to ask with an SS. Weblate wraps long expressions in one line, then script splits it again at about 300 characters length, then this repeats. screenshot-github com-2023 10 18-12_28_20

Although the final result is merely affected since the merge is done on the script's changes, this is still happening on every Weblate commit. (last PR this SS belongs opened about 12 hours ago)

Mark-Giesen commented 10 months ago

This is a YAML example. I thought we were discussing and fixing line wrapping differences between gettext and Weblate, so that's only the PO files. I think those are solved now.

Could it be that WE are doing this ourselves in our pre-commit? That does rewrite our YAML, maybe it does something with long lines aswell? @jtwaleson would know. We instruct Weblate specifically NOT to wrap long lines, and it looks like it doesn't. The commits that we get from Weblate seem to have the long lines, but then an other commit occurs. which wraps these lines.

Mark-Giesen commented 10 months ago

Ah, think I found it, we do this in rewrite-content-yaml.py:

    new_string = yaml.safe_dump(
        data, indent=4, allow_unicode=True, sort_keys=False, width=300
    )

We might have had a reason to do this here @jtwaleson ?

jtwaleson commented 10 months ago

@Mark-Giesen @yilmazdurmaz Sorry for the delay, I've been ill for the last couple of days. There was to be honest no specific reason for the 300, it was a seemingly sensible magic number for the couple of files I looked at manually. Feel free to change this into something that makes more sense!

Mark-Giesen commented 9 months ago

Ah, think I found it, we do this in rewrite-content-yaml.py:

    new_string = yaml.safe_dump(
        data, indent=4, allow_unicode=True, sort_keys=False, width=300
    )

@Felienne and @jpelay this is one other remaining issue with wrapping. YAML files are not wrapped by Weblate, but we wrap them on precommit. YAML and PO files have different issues.

Felienne commented 9 months ago

Ah, think I found it, we do this in rewrite-content-yaml.py:

    new_string = yaml.safe_dump(
        data, indent=4, allow_unicode=True, sort_keys=False, width=300
    )

@Felienne and @jpelay this is one other remaining issue with wrapping. YAML files are not wrapped by Weblate, but we wrap them on precommit. YAML and PO files have different issues.

Yes, the issue we have now is only in po files!