if we edit in Windows and using online UI to submit strings, it results "\r\n" line endings in multiple lines string. so that we are getting 2 SAME strings look exactly same in online UI.

by drupal code standard, it should only use Unix line endings, do it also apply to text strings?

CommentFileSizeAuthor
#11 line_break.patch1.36 KBdroplet
#10 line_break.patch1.36 KBdroplet
#6 newline.patch645 bytesdroplet
#4 newline.patch649 bytesdroplet
#2 l10n_server-991724.patch1011 bytesdroplet

Comments

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

So that would be a submission function to replace Windows line ending with Unix line endings? How can we do an update function for this in a sensible way?

droplet’s picture

droplet commented
StatusFileSize
new l10n_server-991724.patch1011 bytes

here is my patch. does it safe ?

and then require apply it into _l10n_community_import_one_string as well

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Is this the reason for #999092: Import duplicate check broken in fact?

droplet’s picture

droplet commented
Version: 6.x-2.x-dev » 6.x-3.x-dev
Status: Active » Needs review
StatusFileSize
new newline.patch649 bytes

great, we can adding to l10n_community_trim().

good_man’s picture

good_man commented
Status: Needs review » Needs work 
+++ l10n_community/l10n_community.module
@@ -557,6 +557,9 @@ function l10n_community_get_contexts() {
+    $translation = str_replace(array("\r\n", "\r"), "\n", $translation);
+    ¶

Extra tab, you should run your patch on your patch :)

Powered by Dreditor.

droplet’s picture

droplet commented
Status: Needs work » Needs review
StatusFileSize
new newline.patch645 bytes

:)

good_man’s picture

good_man commented

Okay nice simple patch let's wait Gabor.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Looks good. What is going to happen to existing translations which might be wrong?

good_man’s picture

good_man commented

We better need to trim them once and for all, just once. Then will leave it to this patch to filter the new ones.

droplet’s picture

droplet commented
StatusFileSize
new line_break.patch1.36 KB

maybe hook_update, replace all of them once.

droplet’s picture

droplet commented
StatusFileSize
new line_break.patch1.36 KB
good_man’s picture

good_man commented
Status: Needs review » Needs work

Awesome! just a minor issue:

+++ b/l10n_community/l10n_community.module
@@ -557,6 +557,9 @@ function l10n_community_get_contexts() {
+	// Convert line break from CR or CRLF to LF.
+	$translation = str_replace(array("\r\n", "\r"), "\n", $translation);
+

TABS :) replace with spaces.

Powered by Dreditor.

droplet’s picture

droplet commented
Status: Needs work » Needs review

#11 removed the TABS

good_man’s picture

good_man commented

I think it's ready.

SebCorbin’s picture

SebCorbin commented
Status: Needs review » Reviewed & tested by the community

Patch #11 applied, everything is ok for me.

SebCorbin’s picture

SebCorbin commented

I was wondering though, what about the possible duplicates generated by the update?

droplet’s picture

droplet commented

umm. it results two identical strings. but would not broken l10n_server functional or bring conflict error.

from LDO DB(snapshot-125, 04-10-2011), it has 3804 rows contains '\r\n' line breaks and 101 rows duplicated strings (around 10 strings have 3~7 duplicates result.)

SELECT count(translation) AS scount, REPLACE(translation, '\r\n', '\n')
FROM `l10n_server_translation`
WHERE `translation` LIKE '%\r\n%'
GROUP BY translation
HAVING scount > 1
ORDER BY scount DESC

**

Manually way to kill these 101 rows string better than write a update function to solve it I though..

SELECT count( sid ) AS scount, language
FROM `l10n_server_translation`
WHERE `translation` LIKE '%\r\n%'
GROUP BY language
ORDER BY scount DESC

Seems like FR is the most active group to use UI submitting strings

|306|fr
|271|es
|229|sk
|219|hu
|194|pt-br
|182|de
|172|nl
|169|sv
|148|it
|138|pt-pt
|137|da
|130|ja
|111|ca
|96|cs
|89|nb
|74|zh-hant
|67|el
|66|lt
|62|zh-hans
|56|ru
|53|pl
|53|kk
|51|id
|45|uk
|43|lv
|40|tr
|39|hi
|38|fi
|36|sl
|36|vi
|33|ko
|32|cy
|32|ro
|31|hr
|22|sr
|22|ms
|21|et
|20|nn
|20|te
|19|gl
|19|fa
|17|ur
|16|ar
|15|he
|14|ku
|11|mn
|11|ta
|10|af
|8|sq
|8|th
|7|kn
|6|bg
|5|eu
|5|ug
|5|mr
|5|fil
|5|eo
|4|en-gb
|4|is
|4|ml
|3|ky
|3|ne
|3|my
|2|be
|2|fo
|2|km
|1|gsw-berne
|1|dz
|1|test
|1|am
|1|gu
|1|ka
|1|bs
|1|bn
SebCorbin’s picture

SebCorbin commented
Status: Reviewed & tested by the community » Needs work

We need to add the duplicate removal to the update function, as this module is used on other sites.

l10n_community.install from #11 is also missing a new line at end of file