gnuplot-beta

brainstorming SVG_INTERACTIVE

[<pl...@pi...>]

Hi Ethan,

I have the basic mechanics in place to add some interactivity to
svg.trm, so  I need to look at how to integrate this into the rest of
gnuplot.

One of the main needs is to sprinkle a few id=xxx markers into the
various path and group elements so they can be identified and
modified via js and the DOM.

If I want to add another arguement to the functions made visisble in
TERM_TABLE that would seem to mean changing every call and also the
terminals that dont support the new features. While feasible, it seems a
bit radical.

It is generally a good idea to have at least one 'joker' arguement as a
structure pointer in such interdependant interfaces. This allows for
future expansion without altering the prototypes and reorganising the
whole code structure.



This is certainly a result of a long running project that has evolved
well beyond it's original objectives. At some stage it may be worth
adding such extra args.


I had a look at the href patch which seems to work by adding a href
variable to some structures and an SVG_href() function to output the
xlink code. This works quite well for the labels and the command syntax
is clear but it looks less robust for more general entities like plots ,
where trying to wrap it needs several flags to keep track of things.

Re: brainstorming SVG_INTERACTIVE

[Ethan A M. <merritt@u.washington.edu>]

On Monday 29 December 2008, pl...@pi... wrote:
> Hi Ethan,
> 
> I have the basic mechanics in place to add some interactivity to
> svg.trm, so  I need to look at how to integrate this into the rest of
> gnuplot.
> 
> One of the main needs is to sprinkle a few id=xxx markers into the
> various path and group elements so they can be identified and
> modified via js and the DOM.
> 
> If I want to add another arguement to the functions made visisble in
> TERM_TABLE that would seem to mean changing every call and also the
> terminals that dont support the new features. While feasible, it seems a
> bit radical.

You want to add one to every terminal function?
That indeed sounds rather disruptive. Can you not achieve the same thing
by creating a single new entry that passes in a cookie? 

> It is generally a good idea to have at least one 'joker' arguement as a
> structure pointer in such interdependant interfaces. This allows for
> future expansion without altering the prototypes and reorganising the
> whole code structure.

Opinions vary.  The contrary argument is that if you have to put in 
slop variables, this just means that you have not put sufficient thought
into getting the interface design right the first time.

> This is certainly a result of a long running project that has evolved
> well beyond it's original objectives. At some stage it may be worth
> adding such extra args.
> 
> 
> I had a look at the href patch which seems to work by adding a href
> variable to some structures and an SVG_href() function to output the
> xlink code. This works quite well for the labels and the command syntax
> is clear but it looks less robust for more general entities like plots ,
> where trying to wrap it needs several flags to keep track of things.

"A label" maps directly into a single call to the terminal. So it is
easy to attach an additional to it, in this case xlink, and deliver it
to the driver at the right time.  "A plot" contains many, many calls to
the terminal driver.  If a driver needs to track whether it is currently
in the context a plot, or a particular plot, it needs to maintain state
variables.

-- 
Ethan A Merritt
Biomolecular Structure Center
University of Washington, Seattle 98195-7742

Re: brainstorming SVG_INTERACTIVE

[<pl...@pi...>]

Ethan A Merritt wrote:
> On Monday 29 December 2008, pl...@pi... wrote:
>> Hi Ethan,
>>
>> I have the basic mechanics in place to add some interactivity to
>> svg.trm, so  I need to look at how to integrate this into the rest of
>> gnuplot.
>>
>> One of the main needs is to sprinkle a few id=xxx markers into the
>> various path and group elements so they can be identified and
>> modified via js and the DOM.
>>
>> If I want to add another arguement to the functions made visisble in
>> TERM_TABLE that would seem to mean changing every call and also the
>> terminals that dont support the new features. While feasible, it seems a
>> bit radical.
> 
> You want to add one to every terminal function?
> That indeed sounds rather disruptive. Can you not achieve the same thing
> by creating a single new entry that passes in a cookie? 

I used a method like the href patch in the end just to label the legend
text and the plot line. These are fairly well defined as you rightly
point out of the case of labels.

> 
>> It is generally a good idea to have at least one 'joker' arguement as a
>> structure pointer in such interdependant interfaces. This allows for
>> future expansion without altering the prototypes and reorganising the
>> whole code structure.
> 
> Opinions vary.  The contrary argument is that if you have to put in 
> slop variables, this just means that you have not put sufficient thought
> into getting the interface design right the first time.

I agree , but I'm sure over lifetime of a project like gnuplot it's
impossible to even define objectives that will not need changing so by
definition it's impossible to get the interface "right the first time".

Since half your variables are going to be stringz you have to work with
sloppies anyway but I don't see anything sloppy about passing a struct
pointer. Yyou can still have firmly defined vars within the struct it
just saves changing the protos.

Anyway, if you prefer not to I'll leave it there.

> 
>> This is certainly a result of a long running project that has evolved
>> well beyond it's original objectives. At some stage it may be worth
>> adding such extra args.
>>
>>
>> I had a look at the href patch which seems to work by adding a href
>> variable to some structures and an SVG_href() function to output the
>> xlink code. This works quite well for the labels and the command syntax
>> is clear but it looks less robust for more general entities like plots ,
>> where trying to wrap it needs several flags to keep track of things.
> 
> "A label" maps directly into a single call to the terminal. So it is
> easy to attach an additional to it, in this case xlink, and deliver it
> to the driver at the right time.  "A plot" contains many, many calls to
> the terminal driver.  If a driver needs to track whether it is currently
> in the context a plot, or a particular plot, it needs to maintain state
> variables.
> 

I used a method like the href patch in the end just to id the legend
text and the plot line. These are "fairly" well defined as you rightly
point out is the case of labels. This approach is fine for just tacking
on a bit of functionality but don't regard it as a very robust approach.

Anyway, I've hooked that up to something like Bill's toggle code and
I've got what I wanted a gnuplot implementation of Bill's perl hack.

graphics.c

	/* Draw key text in black */
	(*t->linetype)(LT_BLACK);
//*** add legend onclick here
if (term->set_id_click) (term->set_id_click) ("legend#","line#");



//*** force separte path for plot and sample (re svg_interactive)
		closepath();
//*** add id to plot line to toggle***
if (term->set_id_click) (term->set_id_click) ("line#",NULL);

    /* oops - doing the point sample now would break the postscript


The onclick is in svg.js so eventually user accessible without
recompilation.

It's a one off feature so probably could used a syntax like:

set key toggleVisiblitly


Let me know if you have a better idea.

regards, Peter.

Re: brainstorming SVG_INTERACTIVE

[<pl...@pi...>]

pl...@pi... wrote:
> Ethan A Merritt wrote:
>> On Monday 29 December 2008, pl...@pi... wrote:
>>> Hi Ethan,
>>>
>>> I have the basic mechanics in place to add some interactivity to
>>> svg.trm, so  I need to look at how to integrate this into the rest of
>>> gnuplot.
>>>
>>> One of the main needs is to sprinkle a few id=xxx markers into the
>>> various path and group elements so they can be identified and
>>> modified via js and the DOM.
>>>
>>> If I want to add another arguement to the functions made visisble in
>>> TERM_TABLE that would seem to mean changing every call and also the
>>> terminals that dont support the new features. While feasible, it seems a
>>> bit radical.
>> You want to add one to every terminal function?
>> That indeed sounds rather disruptive. Can you not achieve the same thing
>> by creating a single new entry that passes in a cookie? 
> 
> I used a method like the href patch in the end just to label the legend
> text and the plot line. These are fairly well defined as you rightly
> point out of the case of labels.
> 
>>> It is generally a good idea to have at least one 'joker' arguement as a
>>> structure pointer in such interdependant interfaces. This allows for
>>> future expansion without altering the prototypes and reorganising the
>>> whole code structure.
>> Opinions vary.  The contrary argument is that if you have to put in 
>> slop variables, this just means that you have not put sufficient thought
>> into getting the interface design right the first time.
> 
> I agree , but I'm sure over lifetime of a project like gnuplot it's
> impossible to even define objectives that will not need changing so by
> definition it's impossible to get the interface "right the first time".
> 
> Since half your variables are going to be stringz you have to work with
> sloppies anyway but I don't see anything sloppy about passing a struct
> pointer. Yyou can still have firmly defined vars within the struct it
> just saves changing the protos.
> 
> Anyway, if you prefer not to I'll leave it there.
> 
>>> This is certainly a result of a long running project that has evolved
>>> well beyond it's original objectives. At some stage it may be worth
>>> adding such extra args.
>>>
>>>
>>> I had a look at the href patch which seems to work by adding a href
>>> variable to some structures and an SVG_href() function to output the
>>> xlink code. This works quite well for the labels and the command syntax
>>> is clear but it looks less robust for more general entities like plots ,
>>> where trying to wrap it needs several flags to keep track of things.
>> "A label" maps directly into a single call to the terminal. So it is
>> easy to attach an additional to it, in this case xlink, and deliver it
>> to the driver at the right time.  "A plot" contains many, many calls to
>> the terminal driver.  If a driver needs to track whether it is currently
>> in the context a plot, or a particular plot, it needs to maintain state
>> variables.
>>
> 
> I used a method like the href patch in the end just to id the legend
> text and the plot line. These are "fairly" well defined as you rightly
> point out is the case of labels. This approach is fine for just tacking
> on a bit of functionality but don't regard it as a very robust approach.
> 
> Anyway, I've hooked that up to something like Bill's toggle code and
> I've got what I wanted a gnuplot implementation of Bill's perl hack.
> 
> graphics.c
> 
> 	/* Draw key text in black */
> 	(*t->linetype)(LT_BLACK);
> //*** add legend onclick here
> if (term->set_id_click) (term->set_id_click) ("legend#","line#");
> 
> 
> 
> //*** force separte path for plot and sample (re svg_interactive)
> 		closepath();
> //*** add id to plot line to toggle***
> if (term->set_id_click) (term->set_id_click) ("line#",NULL);
> 
>     /* oops - doing the point sample now would break the postscript
> 
> 
> The onclick is in svg.js so eventually user accessible without
> recompilation.
> 
> It's a one off feature so probably could used a syntax like:
> 
> set key toggleVisiblitly
> 
> 
> Let me know if you have a better idea.
> 
> regards, Peter.
> 
> 

OK I've added set key clickable/noclickable cleaned up a bit and done
trivial testing.

Here's the patch. Though I probably missed something out. ;)

Thanks to Bill for the idea and the perl hack to do it.

regards, Peter.

Re: brainstorming SVG_INTERACTIVE

[<pl...@pi...>]

pl...@pi... wrote:
> 
> 
> OK I've added set key clickable/noclickable cleaned up a bit and done
> trivial testing.
> 
> Here's the patch. Though I probably missed something out. ;)
> 
> Thanks to Bill for the idea and the perl hack to do it.
> 
> regards, Peter.
> 
> 
> 

Another nice feature would be a draggable legend (equally valid for
other interactives).

I would also like to see cross-hairs on svg. That would be a great help
in the absence of coordinate display. This is all going t lead to a
desire for a toolbar like the wxt terminal. That would be attainable by
adding a floating box via js.


/Peter.

Re: brainstorming SVG_INTERACTIVE

[Ethan A M. <merritt@u.washington.edu>]

On Tuesday 30 December 2008, pl...@pi... wrote:
> OK I've added set key clickable/noclickable cleaned up a bit and done
> trivial testing.
> 
> Here's the patch. Though I probably missed something out. ;)
> 
> Thanks to Bill for the idea and the perl hack to do it.
> 
> regards, Peter.
> 
> gnuplot_svg_interactive.patch

Please upload it to the SourceForge site:

   https://sourceforge.net/tracker/?group_id=2055&atid=302055

-- 
Ethan A Merritt

Re: brainstorming SVG_INTERACTIVE

[<pl...@pi...>]

Ethan A Merritt wrote:
> On Tuesday 30 December 2008, pl...@pi... wrote:
>> OK I've added set key clickable/noclickable cleaned up a bit and done
>> trivial testing.
>>
>> Here's the patch. Though I probably missed something out. ;)
>>
>> Thanks to Bill for the idea and the perl hack to do it.
>>
>> regards, Peter.
>>
>> gnuplot_svg_interactive.patch
> 
> Please upload it to the SourceForge site:
> 
>    https://sourceforge.net/tracker/?group_id=2055&atid=302055
> 

Patches item #2477391, was opened at 2008-12-30 17:29
Message generated for change (Tracker Item Submitted) made by Item Submitter
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=302055&aid=2477391&group_id=2055


It probably needs tidying up a bit but could you make sure it applies
OK? Made against currect CVS.

Thanks.

Re: brainstorming SVG_INTERACTIVE

[Ethan A M. <merritt@u.washington.edu>]

On Tuesday 30 December 2008, Ethan A Merritt wrote:
> On Tuesday 30 December 2008, pl...@pi... wrote:
> > Ethan A Merritt wrote:
> > > On Tuesday 30 December 2008, pl...@pi... wrote:
> > >> OK I've added set key clickable/noclickable cleaned up a bit and done
> > >> trivial testing.
> > >>
> > >> Here's the patch. Though I probably missed something out. ;)
> > >>
> > >> Thanks to Bill for the idea and the perl hack to do it.
> > >>
> > >> regards, Peter.
> > >>
> > >> gnuplot_svg_interactive.patch
> > > 
> > > Please upload it to the SourceForge site:
> > > 
> > >    https://sourceforge.net/tracker/?group_id=2055&atid=302055
> > > 
> > 
> > Patches item #2477391, was opened at 2008-12-30 17:29
> > Message generated for change (Tracker Item Submitted) made by Item Submitter
> > You can respond by visiting:
> > https://sourceforge.net/tracker/?func=detail&atid=302055&aid=2477391&group_id=2055
> > 
> > 
> > It probably needs tidying up a bit but could you make sure it applies
> > OK? Made against currect CVS.
> 
> 
> First quick comments on the patch:
> 
> 1) Remove all the residual garbage in the patch file caused by comparing
>    the CVS "hidden" files in your own cvs setup.  As it stands, this patch
>    will not apply against anyone else's cvs tree.
> 
> 2) Conversely, the patch does not contain your new .../term/svg subdirectory.
>    For that you would need to include the -N switch in your "diff" command,
>    but it also means that you need to run it on a clean directory tree rather
>    than one that contains all the *.o files and modified CVS directories.
> 
> 3) The patch contains some changes that I assume you made for local
>    convenience (e.g. removing the docs directory from the build dependencies).
>    Please remove these.
> 
> For these various reasons, it doesn't actually apply and "make" correctly.
> I attach a somewhat cleaner version of your patch, but obviously it still
> doesn't contain any of the missing files.
> 
> As to the code itself:
> 
> 1) Please no C++ style comments.  
>    Not everyone is using a C compiler that accepts them.
> 
> 2) Compilation produces the following error messages:
> 
> In file included from term.h:368,
>                  from term.c:1368:
> ../term/svg.trm: In function ‘SVG_PathOpen’:
> ../term/svg.trm:257: warning: ISO C90 forbids mixed declarations and code
> ../term/svg.trm:261: warning: suggest parentheses around assignment used as truth value
> ../term/svg.trm:267: warning: suggest parentheses around assignment used as truth value
> ../term/svg.trm: In function ‘SVG_init’:
> ../term/svg.trm:752: warning: ISO C90 forbids mixed declarations and code
> ../term/svg.trm: In function ‘SVG_put_text’:
> ../term/svg.trm:1134: warning: suggest parentheses around assignment used as truth value
> ../term/svg.trm:1142: warning: suggest parentheses around assignment used as truth value
> 
> unset.c: In function ‘reset_key’:
> unset.c:864: warning: missing braces around initializer
> unset.c:864: warning: (near initialization for ‘temp_key.bounds’)
> 
> 
> 3) I don't get this "event handler" business.  
>    term_api.h:
> 	typedef struct  ext {
> 	  char *id;
> 	  char *event_handler;
> 	} ext ;
> 
>    This obviously is not an event handler in the normal unix sense of the term.
>    Please add a comment explaining exactly what it is, and how various terminal
>    drivers are supposed to use it.   
> 
>    Actaully, it doesn't seem to be used anywhere.
>    Is it a remnant from an earlier version?
> 
> 4) Please document the use of term->set_id_click(), term->move_ex(), and so on.
>    The description must be complete enough so that someone can code up the
>    corresponding routines for other terminals.  
>    It seems that you only ever pass in strings "legend#", "line#", and "sample#".
>    Do these have to be strings at all? Can they be defined constants?   Enums?   
> 
> 5) It would be good to include a demo script, so that people can test the
>    patchset immediately without having to learn the details first.


More comments:

 - "set key clickable" works, but the current setting needs to be reported by
   "show key" and saved by save_set_all().

 - Coding style.  Please make new code conform to the surrounding coding style.
   Rather than 

        /* Draw key text in black */
        (*t->linetype)(LT_BLACK);
+//*** add legend id and onclick
+  if ( (key->clickable) && (term->set_id_click) ) (term->set_id_click) ("legend#","line#");

   Please use

        /* Draw key text in black */
        (*t->linetype)(LT_BLACK);
+       /* add legend id and onclick */
+       if (key->clickable && (*t->set_id_click))
+           (*t->set_id_click)("legend#", "line#");

 - Thinking out loud here ...
   As I understand it, the curent patch associates the action specifically with
   the text of the key title.  What if the plot has no title?
   Maybe it would be better to associate the action with the rectangular block
   that bounds the key title and associated key sample.  That would at least
   hypothetically allow associated the action with an image-mapped *.png plot
   in addition to the more obviously interactive terminal types.

 - I'd like to hear more suggestions or brainstorming about a generalized
   framework.  This patch associates a specific action (toggle on/off) with
   a click on the key title.  My earlier href patch associated a different
   action (link to URL) with a click on the key title.  Another suggestion from
   the first round of comments was to also allow the click to trigger various
   other actions (highlight the plot, erase other plots, grey out the plot).
   
   All of these options sound reasonable, but it means that "set key clickable"
   is not sufficient to specify the desired result.  OK, fine, we could make
   that "set key click=toggle" or "set key click=URL".  But do we need a
   new set of terminal entry points for each option, or can we share a single
   entry that handles all of the various options that apply to that terminal?

   Maybe we need a structure that describes the desired actions, and a 
   corresponding set of keywords to instantiate one.

   struct action {
	int type;    /* toggle, cycle, highlight, hyperlink, ... */
        char *url;   /* NULL unless action type is hyperlink */
        t_colorspec highlight_color;
   }

   struct legend_key {
	...
	struct action *action;
   }

      # default action is to toggle the plot on/off
        set key title action TOGGLE  
      # but individual plots can do something else, 
      # e.g. cycle grey/off/on/highlight 
        plot foo title "foo" action cycle, 
             baz title "baz" action link "file:///data/baz.dat" 



-- 
Ethan A Merritt

Re: brainstorming SVG_INTERACTIVE

[<pl...@pi...>]

Ethan A Merritt wrote:
> On Tuesday 30 December 2008, Ethan A Merritt wrote:
>> On Tuesday 30 December 2008, pl...@pi... wrote:
>>> Ethan A Merritt wrote:
>>>> On Tuesday 30 December 2008, pl...@pi... wrote:
>>>>> OK I've added set key clickable/noclickable cleaned up a bit and done
>>>>> trivial testing.
>>>>>
>>>>> Here's the patch. Though I probably missed something out. ;)
>>>>>
>>>>> Thanks to Bill for the idea and the perl hack to do it.
>>>>>
>>>>> regards, Peter.
>>>>>
>>>>> gnuplot_svg_interactive.patch
>>>> Please upload it to the SourceForge site:
>>>>
>>>>    https://sourceforge.net/tracker/?group_id=2055&atid=302055
>>>>
>>> Patches item #2477391, was opened at 2008-12-30 17:29
>>> Message generated for change (Tracker Item Submitted) made by Item Submitter
>>> You can respond by visiting:
>>> https://sourceforge.net/tracker/?func=detail&atid=302055&aid=2477391&group_id=2055
>>>
>>>
>>> It probably needs tidying up a bit but could you make sure it applies
>>> OK? Made against currect CVS.
>>

> 
> 
> More comments:
> 
>  - "set key clickable" works, but the current setting needs to be reported by
>    "show key" and saved by save_set_all().
> 
>  - Coding style.  Please make new code conform to the surrounding coding style.
>    Rather than 
> 
>         /* Draw key text in black */
>         (*t->linetype)(LT_BLACK);
> +//*** add legend id and onclick
> +  if ( (key->clickable) && (term->set_id_click) ) (term->set_id_click) ("legend#","line#");
> 
>    Please use
> 
>         /* Draw key text in black */
>         (*t->linetype)(LT_BLACK);
> +       /* add legend id and onclick */
> +       if (key->clickable && (*t->set_id_click))
> +           (*t->set_id_click)("legend#", "line#");
> 
>  - Thinking out loud here ...
>    As I understand it, the curent patch associates the action specifically with
>    the text of the key title.  What if the plot has no title?
>    Maybe it would be better to associate the action with the rectangular block
>    that bounds the key title and associated key sample.  That would at least
>    hypothetically allow associated the action with an image-mapped *.png plot
>    in addition to the more obviously interactive terminal types.
Maybe adding a wrapper element would be useful if you want to add an
href but that was not the aim of this patch. Like I said this needs some
top down thinking before it expands, as it surely will.

The sample line is also active with this patch but you need to be a good
shot with the mouse. A larger target would be good.

> 
>  - I'd like to hear more suggestions or brainstorming about a generalized
>    framework.  This patch associates a specific action (toggle on/off) with
>    a click on the key title.  My earlier href patch associated a different
>    action (link to URL) with a click on the key title.  Another suggestion from
>    the first round of comments was to also allow the click to trigger various
>    other actions (highlight the plot, erase other plots, grey out the plot).
>    
>    All of these options sound reasonable, but it means that "set key clickable"
>    is not sufficient to specify the desired result.  OK, fine, we could make
>    that "set key click=toggle" or "set key click=URL".  But do we need a
>    new set of terminal entry points for each option, or can we share a single
>    entry that handles all of the various options that apply to that terminal?
> 
>    Maybe we need a structure that describes the desired actions, and a 
>    corresponding set of keywords to instantiate one.
> 
>    struct action {
> 	int type;    /* toggle, cycle, highlight, hyperlink, ... */
>         char *url;   /* NULL unless action type is hyperlink */
>         t_colorspec highlight_color;
>    }

I'd like to bring the function name onclick=toogleVisible out to a
string that could be "set". Your *url should be free to set anything.
This allows the user to add his own js functions in svg.js and link them
in this way.

This way he has the freedom to do window.parent.location="....." to
break out of frames or whatever. Or pop up an alert() with some info
about the plot. This is very powerful.

It also circumvents the problem of trying to code the <a></a> pair and
leave it dangling while diverse possible code paths happen and then hope
to close it again properly.


> 
>    struct legend_key {
> 	...
> 	struct action *action;
>    }
> 
>       # default action is to toggle the plot on/off
>         set key title action TOGGLE  
>       # but individual plots can do something else, 
>       # e.g. cycle grey/off/on/highlight 
>         plot foo title "foo" action cycle, 
>              baz title "baz" action link "file:///data/baz.dat" 
> 

Yes I like linking to the data.

The current two args are probably not enough but it shows the mechanism.
It probably needs :

id unique id , probably utilising plotno if in the plotting loop
event   [onclick , onmouseenter, ....]
action  [toggleVisibilty, other std, user defined]
target  [null,self,another id]

This shows a limitation on the 'set key' approach, how to define more
than one event handler (onclick , and onmouseover for coords)

set key event 1 onclick
set key action 1 togglevisiblity

??

The first arg will be simple an unique but the second is looking a lot
like a struct* to me. If this is not flexible each new idea is going to
change the code for all terminals.

I'll get back to cleaning up the patch.

regards.

Re: brainstorming SVG_INTERACTIVE

[Ethan A M. <merritt@u.washington.edu>]

On Tuesday 30 December 2008, pl...@pi... wrote:
> Ethan A Merritt wrote:
> > On Tuesday 30 December 2008, pl...@pi... wrote:
> >> OK I've added set key clickable/noclickable cleaned up a bit and done
> >> trivial testing.
> >>
> >> Here's the patch. Though I probably missed something out. ;)
> >>
> >> Thanks to Bill for the idea and the perl hack to do it.
> >>
> >> regards, Peter.
> >>
> >> gnuplot_svg_interactive.patch
> > 
> > Please upload it to the SourceForge site:
> > 
> >    https://sourceforge.net/tracker/?group_id=2055&atid=302055
> > 
> 
> Patches item #2477391, was opened at 2008-12-30 17:29
> Message generated for change (Tracker Item Submitted) made by Item Submitter
> You can respond by visiting:
> https://sourceforge.net/tracker/?func=detail&atid=302055&aid=2477391&group_id=2055
> 
> 
> It probably needs tidying up a bit but could you make sure it applies
> OK? Made against currect CVS.


First quick comments on the patch:

1) Remove all the residual garbage in the patch file caused by comparing
   the CVS "hidden" files in your own cvs setup.  As it stands, this patch
   will not apply against anyone else's cvs tree.

2) Conversely, the patch does not contain your new .../term/svg subdirectory.
   For that you would need to include the -N switch in your "diff" command,
   but it also means that you need to run it on a clean directory tree rather
   than one that contains all the *.o files and modified CVS directories.

3) The patch contains some changes that I assume you made for local
   convenience (e.g. removing the docs directory from the build dependencies).
   Please remove these.

For these various reasons, it doesn't actually apply and "make" correctly.
I attach a somewhat cleaner version of your patch, but obviously it still
doesn't contain any of the missing files.

As to the code itself:

1) Please no C++ style comments.  
   Not everyone is using a C compiler that accepts them.

2) Compilation produces the following error messages:

In file included from term.h:368,
                 from term.c:1368:
../term/svg.trm: In function ‘SVG_PathOpen’:
../term/svg.trm:257: warning: ISO C90 forbids mixed declarations and code
../term/svg.trm:261: warning: suggest parentheses around assignment used as truth value
../term/svg.trm:267: warning: suggest parentheses around assignment used as truth value
../term/svg.trm: In function ‘SVG_init’:
../term/svg.trm:752: warning: ISO C90 forbids mixed declarations and code
../term/svg.trm: In function ‘SVG_put_text’:
../term/svg.trm:1134: warning: suggest parentheses around assignment used as truth value
../term/svg.trm:1142: warning: suggest parentheses around assignment used as truth value

unset.c: In function ‘reset_key’:
unset.c:864: warning: missing braces around initializer
unset.c:864: warning: (near initialization for ‘temp_key.bounds’)


3) I don't get this "event handler" business.  
   term_api.h:
	typedef struct  ext {
	  char *id;
	  char *event_handler;
	} ext ;

   This obviously is not an event handler in the normal unix sense of the term.
   Please add a comment explaining exactly what it is, and how various terminal
   drivers are supposed to use it.   

   Actaully, it doesn't seem to be used anywhere.
   Is it a remnant from an earlier version?

4) Please document the use of term->set_id_click(), term->move_ex(), and so on.
   The description must be complete enough so that someone can code up the
   corresponding routines for other terminals.  
   It seems that you only ever pass in strings "legend#", "line#", and "sample#".
   Do these have to be strings at all? Can they be defined constants?   Enums?   

5) It would be good to include a demo script, so that people can test the
   patchset immediately without having to learn the details first.


-- 
Ethan A Merritt

Re: brainstorming SVG_INTERACTIVE

[<pl...@pi...>]

Ethan A Merritt wrote:
> On Tuesday 30 December 2008, pl...@pi... wrote:
>> Ethan A Merritt wrote:
>>> On Tuesday 30 December 2008, pl...@pi... wrote:
>>>> OK I've added set key clickable/noclickable cleaned up a bit and done
>>>> trivial testing.
>>>>
>>>> Here's the patch. Though I probably missed something out. ;)
>>>>
>>>> Thanks to Bill for the idea and the perl hack to do it.
>>>>
>>>> regards, Peter.
>>>>
>>>> gnuplot_svg_interactive.patch
>>> Please upload it to the SourceForge site:
>>>
>>>    https://sourceforge.net/tracker/?group_id=2055&atid=302055
>>>
>> Patches item #2477391, was opened at 2008-12-30 17:29
>> Message generated for change (Tracker Item Submitted) made by Item Submitter
>> You can respond by visiting:
>> https://sourceforge.net/tracker/?func=detail&atid=302055&aid=2477391&group_id=2055
>>
>>
>> It probably needs tidying up a bit but could you make sure it applies
>> OK? Made against currect CVS.
> 
> 
> First quick comments on the patch:
> 
> 1) Remove all the residual garbage in the patch file caused by comparing
>    the CVS "hidden" files in your own cvs setup.  As it stands, this patch
>    will not apply against anyone else's cvs tree.
> 
> 2) Conversely, the patch does not contain your new .../term/svg subdirectory.
>    For that you would need to include the -N switch in your "diff" command,
>    but it also means that you need to run it on a clean directory tree rather
>    than one that contains all the *.o files and modified CVS directories.
Oops, -auN .

> 
> 3) The patch contains some changes that I assume you made for local
>    convenience (e.g. removing the docs directory from the build dependencies).
>    Please remove these.
> 
> For these various reasons, it doesn't actually apply and "make" correctly.
> I attach a somewhat cleaner version of your patch, but obviously it still
> doesn't contain any of the missing files.
> 
> As to the code itself:
> 
> 1) Please no C++ style comments.  
>    Not everyone is using a C compiler that accepts them.
Check,

> 
> 2) Compilation produces the following error messages:

Well warnings. OK, I've cleaned them up.

> 
> In file included from term.h:368,
>                  from term.c:1368:
> ../term/svg.trm: In function ‘SVG_PathOpen’:
> ../term/svg.trm:257: warning: ISO C90 forbids mixed declarations and code
> ../term/svg.trm:261: warning: suggest parentheses around assignment used as truth value
> ../term/svg.trm:267: warning: suggest parentheses around assignment used as truth value
> ../term/svg.trm: In function ‘SVG_init’:
> ../term/svg.trm:752: warning: ISO C90 forbids mixed declarations and code
> ../term/svg.trm: In function ‘SVG_put_text’:
> ../term/svg.trm:1134: warning: suggest parentheses around assignment used as truth value
> ../term/svg.trm:1142: warning: suggest parentheses around assignment used as truth value
> 
> unset.c: In function ‘reset_key’:
> unset.c:864: warning: missing braces around initializer
> unset.c:864: warning: (near initialization for ‘temp_key.bounds’)
> 

Can't suss that last one...

> 
> 3) I don't get this "event handler" business.  
>    term_api.h:
> 	typedef struct  ext {
> 	  char *id;
> 	  char *event_handler;
> 	} ext ;
> 
>    This obviously is not an event handler in the normal unix sense of the term.
>    Please add a comment explaining exactly what it is, and how various terminal
>    drivers are supposed to use it.   
> 
>    Actaully, it doesn't seem to be used anywhere.
>    Is it a remnant from an earlier version?

It was the js eventhandler for onclick . I originally wanted to extend
the argument list with a struct pointer as I suggested but you weren't
keen. In which case this is obselete.

> 
> 4) Please document the use of term->set_id_click(), term->move_ex(), and so on.
>    The description must be complete enough so that someone can code up the
>    corresponding routines for other terminals.  
>    It seems that you only ever pass in strings "legend#", "line#", and "sample#".
>    Do these have to be strings at all? Can they be defined constants?   Enums?   

For the purposes of this functionality yes, future js may want actual
variables here so I left it open.

> 
> 5) It would be good to include a demo script, so that people can test the
>    patchset immediately without having to learn the details first.
> 


Yes a demo would be good, I just wanted to make sure it was in an
acceptable form before going to that sort of lengths.

I wanted to get this checked a bit before submitting to SF but you
requested I post it. Once you're happy with the state of it I'll resubmit.

Thanks for your comments.

Peter.

Re: brainstorming SVG_INTERACTIVE

[Ethan A M. <merritt@u.washington.edu>]

On Tuesday 30 December 2008, pl...@pi... wrote:

> It was the js eventhandler for onclick . I originally wanted to extend
> the argument list with a struct pointer as I suggested but you weren't
> keen.

You misunderstood me. I was not objecting to passing a structure pointer; 
I think that is a good idea.  I was just pointing out that extra
parameters added "for slop" are not a great recommendation for a design.

> > 4) Please document the use of term->set_id_click(), term->move_ex(), and so on.
> >    The description must be complete enough so that someone can code up the
> >    corresponding routines for other terminals.  
> >    It seems that you only ever pass in strings "legend#", "line#", and "sample#".
> >    Do these have to be strings at all? Can they be defined constants?   Enums?   
> 
> For the purposes of this functionality yes, future js may want actual
> variables here so I left it open.

But the core code cannot pass anything that is specific to a particular
terminal driver.  Anything that refers to jscript or svg tags must be
generated by the driver from generic requests passed in from the core.
It's OK to pass a URL as a string so that the driver can wrap it in 
whatever code is needed to trigger that link.  But it's not OK to have
the core code pass in a fragment of xml or jscript or svg or whatever.
The same call has to work for other terminal drivers.


> > 5) It would be good to include a demo script, so that people can test the
> >    patchset immediately without having to learn the details first.
> > 
> Yes a demo would be good, I just wanted to make sure it was in an
> acceptable form before going to that sort of lengths.

You'll get more testers if you make easy to test.
The easiest test is to run a script that is supposed to work.
If the tester has to roll his own, and it doesn't work, then he
doesn't know whether the code is failing or if he misunderstood
how to use it, or what.

Even if you post a sample output plot, it is good to also provide the
script that generated it. For instance, in the plot you posted before,
"150tank-test.svg", only 2 of the 9 plots listed in the key are visible.
Those two can be toggled off, but the other 7 cannot be toggled on.
Is this intended?  Is it a bug?  Hard to say without seeing the script.

> I wanted to get this checked a bit before submitting to SF but you
> requested I post it. Once you're happy with the state of it I'll resubmit.



-- 
Ethan A Merritt

Re: brainstorming SVG_INTERACTIVE

[<pl...@pi...>]

Ethan A Merritt wrote:
> On Tuesday 30 December 2008, pl...@pi... wrote:
> 
>> It was the js eventhandler for onclick . I originally wanted to extend
>> the argument list with a struct pointer as I suggested but you weren't
>> keen.
> 
> You misunderstood me. I was not objecting to passing a structure pointer; 
> I think that is a good idea.  I was just pointing out that extra
> parameters added "for slop" are not a great recommendation for a design.
> 
>>> 4) Please document the use of term->set_id_click(), term->move_ex(), and so on.
>>>    The description must be complete enough so that someone can code up the
>>>    corresponding routines for other terminals.  
>>>    It seems that you only ever pass in strings "legend#", "line#", and "sample#".
>>>    Do these have to be strings at all? Can they be defined constants?   Enums?   
>> For the purposes of this functionality yes, future js may want actual
>> variables here so I left it open.
> 
> But the core code cannot pass anything that is specific to a particular
> terminal driver.  Anything that refers to jscript or svg tags must be
> generated by the driver from generic requests passed in from the core.
> It's OK to pass a URL as a string so that the driver can wrap it in 
> whatever code is needed to trigger that link.  But it's not OK to have
> the core code pass in a fragment of xml or jscript or svg or whatever.
> The same call has to work for other terminal drivers.
> 
I realise that , there will be some generics that several terminals can
do but that should not cripple what svg is capable of.

Adding a link effectively means adding an onclick event , that is
possible in latex svg pdf, possibly bitmapped. SVG offers the
possiblitly to add the interactivity of the current real-time
interactives but after the fact and in a distribuatable form. More over
much of that functionality can be user determined not limitted to what
is compiled into gnuplot.

I think the generic approach should be pushed as far as possible without
killing the power of what svg offers that the others never can.

That would seem to be in keeping with the current philosophy.


> 
>>> 5) It would be good to include a demo script, so that people can test the
>>>    patchset immediately without having to learn the details first.
>>>
>> Yes a demo would be good, I just wanted to make sure it was in an
>> acceptable form before going to that sort of lengths.
> 
> You'll get more testers if you make easy to test.
> The easiest test is to run a script that is supposed to work.
> If the tester has to roll his own, and it doesn't work, then he
> doesn't know whether the code is failing or if he misunderstood
> how to use it, or what.
> 
> Even if you post a sample output plot, it is good to also provide the
> script that generated it. For instance, in the plot you posted before,
> "150tank-test.svg", only 2 of the 9 plots listed in the key are visible.
> Those two can be toggled off, but the other 7 cannot be toggled on.
> Is this intended?  Is it a bug?  Hard to say without seeing the script.

Which is why I did not post the script , I sent the first rough to the
list so your could see it work. Its not a demo file.

> 
>> I wanted to get this checked a bit before submitting to SF but you
>> requested I post it. Once you're happy with the state of it I'll resubmit.
> 
> 
> 

/P