processTemplate

[sgillies@…]

MapScript should support multiple class styles, this requires methods to get
styles from the array in the parent classObj, and a method to add new styles
to the array.  

Currently, the way this is done for layerObj and classObj instances is
like so:

   new_classobj = mapscript.classObj(layerobj)

The "parent" node is passed to the class constructor as the single argument.

I really disagree with this usage and think that for the styleObj it should
instead be:

   new_style = mapscript.styleObj()   # created without a parent
   classobj.appendStyle(new_style)    # explicitly add it to the parent

If the styleObj constructor takes _any_ argument at all, it should be a
map file fragment like

   new_style = mapscript.styleObj('STYLE\nCOLOR 0 0 0\nEND')

This is just my opinion, and breaks from the old way, but could lead to
better classes in the future.  I'd like to get some feedback before I
start.

[sdlime]

What you propose is fine. I was thinking that you're right, it's not consistent 
with other objects but styles are more useful than a class. You might have a 
general "red" style that you could use across layers. I like the argument idea 
but it's too soon for that. We might also consider an insertStyle method to put 
a style someplace in the style stack. It would take an integer as an argument, 
a value of 0 or less puts it at the begining of the stack, a value larger than 
the stack size puts it at the end. May be overkill for most users though.

Steve

[sgillies@…]

Steve, I agree that the map file fragment arg is not something that we
will soon realize, but I'd like to "reserve" the option for it :)

insertStyle() would be useful, and perhaps classObj methods to
promote (increase index) and demote (decrease index) styles within
the array.

[sgillies@…]

OK, have implemented a styleObj shadow class, and extended classObj with
getStyle, insertStyle, and removeStyle methods.  Also added a new error
type for errors involving access of the layers, classes, styles arrays
in MapServer, MS_CHILDERR and it's Python exception MapServerChildError.

Some example code from the tests:

Inserting a new style:

    l_layer = self.mapobj1.getLayerByName('LINE')
    class0 = l_layer.getClass(0)
    new_style = styleObj()
    # second arg of insertStyle is optional, defaults to -1
    # or the tail of styles
    index = class0.insertStyle(new_style, 0) # insert at head of styles

Removing a style:

    p_layer = self.mapobj1.getLayerByName('POINT')
    class0 = p_layer.getClass(0)
    # return a copy of the style removed from the provided index
    rem_style = class0.removeStyle(1)

Modify a style:

    p_layer = self.mapobj1.getLayerByName('POINT')
    class0 = p_layer.getClass(0)
    style1 = class0.getStyle(1)        # Just like classObj::getLayer
    style1.color.setRGB(255, 255, 0)

Would be really nice to be able to insert references to a styleObj into
a class, but that would mean a refactor of classObj->styles to be type
styleObj**, requiring a lot of code changes.  One thing to look out for
in the future is that insertStyle and removeStyle depend on msCopyStyle,
which requires manual maintenance in the case of changes to styleObj.

[dmorissette]

Guys, please keep us posted when you add stuff in mapscript.i so that we can
apply the same changes to php_mapscript.  

Or even better: create a corresponding bug in the "MapScript-PHP" component with
a reference to the bug describing the mapscript.i changes.  I have done that
now: created bug 555 to implement those new methods in php_mapscript.

Thanks!

Daniel

[sgillies@…]

Reopening to work on better integration with code in maputil.c.

Anyone mind if I add some calls to msSetError(MS_CHILDERR,)
into msMoveStyleUp and msMoveStyleDown?  

[sgillies@…]

New code introduced in revision 1.137 mapscript.i results in these
errors when trying to compile mapscript_wrap.c:

mapscript_wrap.c:920: warning: function declaration isn't a prototype
mapscript_wrap.c:1243: warning: function declaration isn't a prototype
mapscript_wrap.c:1371: redefinition of `self'
mapscript_wrap.c:1371: `self' previously declared here
mapscript_wrap.c: In function `classObj_moveStyleUp':
mapscript_wrap.c:1371: redeclaration of `self'
mapscript_wrap.c:1371: `self' previously declared here
mapscript_wrap.c: At top level:
mapscript_wrap.c:1374: redefinition of `self'
mapscript_wrap.c:1374: `self' previously declared here
mapscript_wrap.c: In function `classObj_moveStyleDown':
mapscript_wrap.c:1374: redeclaration of `self'
mapscript_wrap.c:1374: `self' previously declared here
mapscript_wrap.c: At top level:
mapscript_wrap.c:1545: redefinition of `self'
mapscript_wrap.c:1545: `self' previously declared here
mapscript_wrap.c: In function `layerObj_applySLD':
mapscript_wrap.c:1545: redeclaration of `self'

Let's don't commit code that doesn't compile!

[sdlime]

Who added the code?

[assefa]

Sorry about that. I am not well setup to test the SWIG versions but try to keep
php/mapscript and other swig versions in sync. I will update the CVS version. If
you have any problems, let me know.

[sgillies@…]

Dunno, but I've almost got it fixed.

Was just a misunderstanding about how to write for SWIG, and then no follow
up with swig -lang mapscript.i; cc -c mapscript_wrap.c to test it before
the commit.

[sgillies@…]

Well, it looks like Assefa and I started fixing this at the same time. Doh.
I hope my changes stick.

This really points out a problem with trying to keep the SWIG and PHP
versions of MapScript in synch ...

PHP-MapScript developers are creating cool new functions that we need in
the SWIG mapscript -- but aren't set up to SWIG the interface file and
compile the wrapper, and aren't set up to run the unit tests under 
python (which are the most extensive regression tests that are included
in the mapserver distribution).

SWIG-MapScript developers are also creating cool new functions, but aren't
too invested in writing the low-level C PHP code to implement them for
PHP-mapscript and aren't set up to do regression tests with PHP.

I dunno if moving PHP mapscript to SWIG is the answer ... I love SWIG
and all, but I'm also getting tired of some of its limitations -- such
as lack of documentation support.

Anyhow, I did make the fixes so that mapscript compiles.  I am getting a 
malloc error from one of my tests, but I'll have that fixed soon as well.

[sgillies@…]

This bug is closed.  Please stay out of mapscript.i and maputil.c
while I resolve the conflicts. :)

[dmorissette]

I agree with Sean's comment that there is a bit of a problem with the PHP vs
SWIG versions of MapScript.  Unfortunately I don't think we can move the PHP
version to use SWIG yet since the PHP support didn't seem very mature last time
we checked... and that would take time anyway... we may try that when we go to
PHP5, we'll see.

Anyway, in the meantime I think the solution is good communication, and bugzilla
can help us for that.  

We (I and Asefa) should create SWIG-MapScript bugs when we add stuff to
PHP-MapScript so that you can port/test the changes, and you (Steve,Sean) should
create PHP-MapScript bugs when you add stuff to SWIG-MapScript for us to
port/test the changes.  Emails can work too, but email often get lost... OTOH
bugs can't go away until they are marked fixed.

[assefa]

I would also mention that we should try as much as possible put most of the
logic used in mapscript functions into one of mapserver c files so we only end
up having a function call to implement in the mapscript.

[tmelhuish@…]

Sean,

I download the nightly build on Monday 2/9/04 to try out the styleObj fix. 
Here is the commands I used and nothing appeared on the screen and I get no 
error messages in the apache log.

        ${"Class"."$ctr"}= new mapscript::classObj($layer);
	$Style= new mapscript::styleObj();		
	$Color= new mapscript::colorObj();
	${"Class"."$ctr"}->{styles}=$Style;
	$Style->{color}=$Color;
	$Color->{red}= int($r);
	$Color->{green}= int($g);
	$Color->{blue}= int($b);

I also tried $Style=${"Class"."$ctr"}->getStyles(0) after I created as new 
Class and the get command is not a valid method. 

I'm assuming your changes noted in this bug report are not in the nightly 
build. If not, can you attach the code I need to compile to test this out in 
mapscript.

Thanks,
tom 

[tmelhuish@…]

I wasn't using your new methods - insertStyle or getStyle. It works fine and 
appriciate all of your work and effort.

Thanks again
tom

[sgillies@…]

Update: I've added an optional classObj argument to the styleObj constructor
so that it can be used in two ways:

    new_style = mapscript.styleObj(the_class)

or

    new_style = mapscript.styleObj()
    the_class.insertStyle(new_style, style_index)

I've got a number of uses for stand-alone styleObjs and would like to do this
for layerObjs at some future time.

[dmorissette]

Adding "port-2-php" keyword.