Differential D14311

Add a project configuration menu entry for the Word Wrap feature
ClosedPublic
Actions

Authored by sdepiets on Jul 24 2018, 3:25 AM.

Details

Reviewers
aacid
ltoscano
shaforostoff
mlaurent
huftis
Group Reviewers
Localization
Commits
R456:84e6644d4fc7: Add a project configuration menu entry for the Word Wrap feature
R456:b1988488c00a: Add a project configuration menu entry for the Word Wrap feature
R456:2c9be462edc2: Add a project configuration menu entry for the Word Wrap feature
Summary

There is a word wrap flag in the project.lokalize file but there was no ui widget to change it.

FEATURE: 396774

Diff Detail

Repository
R456 Lokalize
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sdepiets requested review of this revision.Jul 24 2018, 3:25 AM
sdepiets created this revision.
sdepiets retitled this revision from Add a menu entry for the Word Wrap feature to Add a project configuration menu entry for the Word Wrap feature.Jul 24 2018, 3:27 AM
sdepiets edited the summary of this revision. (Show Details)
sdepiets added reviewers: Localization, aacid, ltoscano, shaforostoff, mlaurent, huftis.
sdepiets added a project: Localization.
sdepiets added a comment.Jul 27 2018, 2:14 AM

I'll push this next week if there is no additional comments

aacid accepted this revision.Jul 29 2018, 9:31 PM
This revision is now accepted and ready to land.Jul 29 2018, 9:31 PM
sdepiets closed this revision.Jul 30 2018, 1:38 AM
huftis reopened this revision.Jul 30 2018, 11:36 AM

I hadn’t have a chance to take a look at this before now. The feature for disabling word-wrapping is very welcome (it will lead to much cleaner SVN diffs), but the word-wrapping in general seems buggy for small (positive) numbers. If I set the word-wrapping to 3 and save the PO file (I tested this with lokalize.po), Lokalize promptly 1) deletes to contents of the PO file, 2) starts using 100% CPU and eating more and more memory, until 3) my system runs out of memory and freezes/crashes (needing a hard reboot).

This revision is now accepted and ready to land.Jul 30 2018, 11:36 AM
sdepiets added a comment.Jul 31 2018, 2:52 AM
In D14311#300531, @huftis wrote:

I hadn’t have a chance to take a look at this before now. The feature for disabling word-wrapping is very welcome (it will lead to much cleaner SVN diffs), but the word-wrapping in general seems buggy for small (positive) numbers. If I set the word-wrapping to 3 and save the PO file (I tested this with lokalize.po), Lokalize promptly 1) deletes to contents of the PO file, 2) starts using 100% CPU and eating more and more memory, until 3) my system runs out of memory and freezes/crashes (needing a hard reboot).

There is indeed a bug in the wrapper feature; the size of the word wrap length is reduced by 1 everytime a \\ character is encountered in order not to cut a \n in a string for instance.
So if you start with a 6 length, you'll reduce the length to 5 on the first \\, then 4, 3, 2, 1, 0. Then infinite loop @ 0.

https://github.com/KDE/lokalize/blob/master/src/catalog/gettext/gettextexport.cpp

sdepiets updated this revision to Diff 38820.Jul 31 2018, 2:59 AM

Fix a bug in the \\ escaping

sdepiets closed this revision.Jul 31 2018, 3:05 AM
sdepiets reopened this revision.Jul 31 2018, 3:14 AM
This revision is now accepted and ready to land.Jul 31 2018, 3:14 AM
sdepiets updated this revision to Diff 38822.Jul 31 2018, 3:14 AM

Disable word wrap at or below 3 characters

Closed by commit R456:2c9be462edc2: Add a project configuration menu entry for the Word Wrap feature (authored by sdepiets). · Explain WhyJul 31 2018, 3:15 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 38823

View Options

src/catalog/gettext/gettextexport.cpp

Loading...