wesnoth-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Wesnoth-patches] [patch #3572] Improvement/fix to parsing of parenthese


From: Isaac Dupree
Subject: [Wesnoth-patches] [patch #3572] Improvement/fix to parsing of parentheses( ( , ) ) in macro arguments
Date: Sun, 19 Dec 2004 15:54:45 -0500
User-agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/125.4 (KHTML, like Gecko) Safari/125.9

This mail is an automated notification from the patch tracker
 of the project: Battle for Wesnoth.

/**************************************************************************/
[patch #3572] Latest Modifications:

Changes by: 
                Isaac Dupree <address@hidden>
'Date: 
                Sun 12/19/2004 at 20:12 (GMT)

------------------ Additional Follow-up Comments ----------------------------
I personally think my patch is at least as good as the current code, although 
both of them have their faults.  When I saw the current implementation, I 
didn't like it much and couldn't figure out a simpler way to fix my problems 
with it.

If you check for quotes everywhere, I don't see how it can be done much better 
than how my patch does things.  Checking for matching quotes would be a 
reasonable idea if by itself it solved the problems without much effort.

The main reason I did this was not because of quotes, but because of the bug I 
referenced: it is currently impossible to do, for example, this, without 
manually expanding one of the macros or doing something else weird with it:
{MACRO ({MACRO2 ([kill]
fire_event=yes
animate=yes
[/kill]) yes})}

Although multiple-line macro usages are pretty bad too, sometimes they're the 
best way to do things.

Although I understand that you have some concern here, I don't know what is 
wrong with my patch, what I can do to fix it, or whether there exists such a 
patch that would be acceptable to you.  Do you think the bug I have referenced 
should even be fixed, or would it inherently make the code too complicated? (of 
course, I suppose no-one can know whether there exists a perfect patch that 
no-one has thought of yet...)






/**************************************************************************/
[patch #3572] Full Item Snapshot:

URL: <http://savannah.nongnu.org/patch/?func=detailitem&item_id=3572>
Project: Battle for Wesnoth
Submitted by: Isaac Dupree
On: Tue 12/14/2004 at 20:23

Category:  None
Priority:  5 - Normal
Resolution:  None
Privacy:  Public
Assigned to:  None
Originator Email:  
Status:  Open


Summary:  Improvement/fix to parsing of parentheses( ( , ) ) in macro arguments

Original Submission:  It is basically a rewrite of parse_macro_arguments().  
This fixes bug #10995 (which I submitted before I got an account) and tries to 
do the best job for various situations.  My code's comments explain why it does 
some of the more mysterious parts of the code and the only situation I could 
think of where it wouldn't work, which would probably never happen, doesn't 
have a good solution, and would be hard-to-understand WML anyway.

Other particular improvements over the current code are:
* You can specify a totally empty macro argument with {MACRO ()}; the current 
code would interpret that argument as a single space.
* Consecutive spaces in parenthesized arguments are preserved: {MACRO (Several 
spaces:    .)}.  The current code (as well as HTML -- ugh) reduces the string 
of spaces in the argument to one space, but my code will preserve it.

Follow-up Comments
------------------


-------------------------------------------------------
Date: Sun 12/19/2004 at 20:12       By: Isaac Dupree <invisibleph>
I personally think my patch is at least as good as the current code, although 
both of them have their faults.  When I saw the current implementation, I 
didn't like it much and couldn't figure out a simpler way to fix my problems 
with it.

If you check for quotes everywhere, I don't see how it can be done much better 
than how my patch does things.  Checking for matching quotes would be a 
reasonable idea if by itself it solved the problems without much effort.

The main reason I did this was not because of quotes, but because of the bug I 
referenced: it is currently impossible to do, for example, this, without 
manually expanding one of the macros or doing something else weird with it:
{MACRO ({MACRO2 ([kill]
fire_event=yes
animate=yes
[/kill]) yes})}

Although multiple-line macro usages are pretty bad too, sometimes they're the 
best way to do things.

Although I understand that you have some concern here, I don't know what is 
wrong with my patch, what I can do to fix it, or whether there exists such a 
patch that would be acceptable to you.  Do you think the bug I have referenced 
should even be fixed, or would it inherently make the code too complicated? (of 
course, I suppose no-one can know whether there exists a perfect patch that 
no-one has thought of yet...)

-------------------------------------------------------
Date: Sat 12/18/2004 at 09:34       By: Guillaume Melquiond <silene>
Hmmm... I don't approve of this patch. WML parsing rules are already awful, no 
need for them to become even worse. I'm all for the quotes hiding parentheses, 
and the quotes having hence to be balanced. Especially now that we allow string 
concatenation. So {MACROX ("abc) (def")} should really be a one-argument macro. 
And not only quotes, but also () and {} outside of quotes should be balanced in 
macro arguments.






File Attachments
-------------------

-------------------------------------------------------
Date: Tue 12/14/2004 at 20:23  Name: config-cpp.diff  Size: 4.32KB   By: 
invisibleph

http://savannah.nongnu.org/patch/download.php?item_id=3572&amp;item_file_id=3945






For detailed info, follow this link:
<http://savannah.nongnu.org/patch/?func=detailitem&item_id=3572>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/







reply via email to

[Prev in Thread] Current Thread [Next in Thread]