After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 606838 - const triple quoted strings are too long
const triple quoted strings are too long
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Code Generator
unspecified
Other Linux
: Normal blocker
: ---
Assigned To: Vala maintainers
Vala maintainers
: 539894 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-01-13 12:40 UTC by pancake
Modified: 2010-10-14 08:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Split large string literal in multiple lines (2.19 KB, patch)
2010-01-22 23:11 UTC, Marc-Andre Lureau
none Details | Review
Split large string literal in multiple lines (2.20 KB, patch)
2010-01-25 19:42 UTC, Marc-Andre Lureau
needs-work Details | Review
Split large string literal in multiple lines (2.39 KB, patch)
2010-03-10 21:46 UTC, Marc-Andre Lureau
none Details | Review
Split large string literal in multiple lines (2.32 KB, patch)
2010-03-14 17:07 UTC, Marc-Andre Lureau
none Details | Review

Description pancake 2010-01-13 12:40:53 UTC
When using """ strings, the generated code in C should be:

var str = """
this is just
a random text
to test quoted
strings
""";

const char *str = "\n" 
   "this is just\n"
   "a random text\n"
   "to test quoted\n"
   "strings\n";

And the same for const strings (#define)

The string in C should be parsed to ensure a decent column width:
  - split string at every \n
  - split string if text exceeds column N

The problem is that Visual Studio does not supports long lines and fails to compile such C files.

It also makes the C code hard to read.
Comment 1 Jürg Billeter 2010-01-14 11:57:59 UTC
Does the above example already fail with Visual Studio or does it only fail at longer lines? What's the limit?
Comment 2 Xavi Artigas 2010-01-14 12:06:38 UTC
It fails when lines are longer than 16Kb, according to:
http://msdn.microsoft.com/en-us/library/dddywwsc(VS.80).aspx
Comment 3 Marc-Andre Lureau 2010-01-22 23:11:29 UTC
Created attachment 152051 [details] [review]
Split large string literal in multiple lines

Fixes bug 606838.
Comment 4 pancake 2010-01-25 12:59:45 UTC
This patch is ok for normal variables, but if you set the variable as const, vala generates a CPP macro which needs a '\' at the end of the line to not chop the macro at the first line. Here's a example:

 264 #define FOO_preference_window_string "\n"
 265 "<?xml version=\"1.0\"?>\n"
 266 "<!--Generated with glade3 3.4.5 on Tue Dec 22 10:52:57 2009 -->\n"
 267 "<interface>\n"
..

must be:

 264 #define FOO_preference_window_string "\n" \
 265 "<?xml version=\"1.0\"?>\n" \
 266 "<!--Generated with glade3 3.4.5 on Tue Dec 22 10:52:57 2009 -->\n" \
 267 "<interface>\n" \
..

Another enhacement would be to check if the string between '\n' is longer than X and then split it again without appending "\n" to ensure that no line is that long.
Comment 5 Marc-Andre Lureau 2010-01-25 14:23:12 UTC
(In reply to comment #4)

> must be:
> 
>  264 #define FOO_preference_window_string "\n" \
>  265 "<?xml version=\"1.0\"?>\n" \
>  266 "<!--Generated with glade3 3.4.5 on Tue Dec 22 10:52:57 2009 -->\n" \
>  267 "<interface>\n" \
> ..

Should we add \ in all cases, that would be a trivial change.

> Another enhacement would be to check if the string between '\n' is longer than
> X and then split it again without appending "\n" to ensure that no line is that
> long.

that I do already, or could you rephrase?
Comment 6 pancake 2010-01-25 15:50:50 UTC
Yep, it's ok to append \ everywhere.

Oh, sorry, you are right, your patch is checking for strings longer than 68 chars (maybe a longer size can be ok too) it depends on how many columns do vala standards recommend :)
Comment 7 Marc-Andre Lureau 2010-01-25 16:06:32 UTC
(In reply to comment #6)
> Yep, it's ok to append \ everywhere.
> 

ok, I'll update the patch

> Oh, sorry, you are right, your patch is checking for strings longer than 68
> chars (maybe a longer size can be ok too) it depends on how many columns do
> vala standards recommend :)

it's not really 68, it's more like 70, if character 68 is a backslash (which is not already escaped), it takes the following char, and then put a quote = 70 char ;) well, I don't know, I didn't though too much about it.
Comment 8 Marc-Andre Lureau 2010-01-25 19:42:09 UTC
Created attachment 152253 [details] [review]
Split large string literal in multiple lines

Fixes bug 606838.
Comment 9 Jürg Billeter 2010-02-11 20:57:39 UTC
This breaks multi-character escape sequences such as in the following example:

void main () {
	var s = "lorem ipsum dolor sit amet, consectetur, sadipisci velit lorem ipsu\x35 lorem ipsum";
	message (s);
}
Comment 10 Marc-Andre Lureau 2010-03-10 21:46:41 UTC
Created attachment 155791 [details] [review]
Split large string literal in multiple lines

Fixes bug 606838.
Comment 11 Marc-Andre Lureau 2010-03-14 17:07:27 UTC
Created attachment 156127 [details] [review]
Split large string literal in multiple lines

Fixes bug 606838.
Comment 12 Jürg Billeter 2010-03-14 18:36:18 UTC
I'm afraid my above testcase still fails with the latest patch. I'll try to come up with a fix.
Comment 13 Jürg Billeter 2010-03-14 19:32:05 UTC
commit 20666769c446ece07ef1063188d3ac5879c9f84c
Author: Jürg Billeter <j@bitron.ch>
Date:   Sat Jan 23 00:03:06 2010 +0100

    Split large string literals into multiple lines
    
    Based on patch by Marc-André Lureau, fixes bug 606838.
Comment 14 Jürg Billeter 2010-10-14 08:28:40 UTC
*** Bug 539894 has been marked as a duplicate of this bug. ***