cashew-s-editor-patches
[Top][All Lists]
Advanced

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

RE: [CASHeW-s-editor-patches] I am commiting changes to supportconsisten


From: Andrew John Hughes
Subject: RE: [CASHeW-s-editor-patches] I am commiting changes to supportconsistent behavour (starting from max always) of editor.......
Date: Thu, 31 Mar 2005 21:40:07 +0100

On Thu, 2005-03-31 at 20:01 +0100, R Bhagdev wrote:
> Well, Andrew what u did in the factory initally was just compare in the list 
> if
> the new name is there or not and disallow it if its not there.  It was about
> three line function returning boolean value.  This meant if the node is
> deleted, it will allow the same name when the editor is restarted.

You're referring to the registrar there, not the factory (which I
described  as a hack that should be fixed to do the comparison using the
diagram list).  And, yes, deletion wasn't adapted, but it's hardly a
major undertaking to do so.

What I'm referring to is getNewObject(String) in the NodeFactory:

  public Node getNewObject(String name)
  {
    Pattern untitled = Pattern.compile("Untitled([0-9]*)");
    Matcher matcher = untitled.matcher(name);
    if (matcher.matches())
      {
        int serializedId = Integer.parseInt(matcher.group());
        autoId = serializedId + 1;
      }
    node = createNode(name);
    if (!NodeRegistrar.registerNewNode(node.getName()))
      {
        throw new IllegalStateException("A node with one of the "
                                        + "deserialized names already "
                                        + "exists.");
      }
    return node;
  }

autoId should (this is untested, as I said) be the id found in the
untitled node + 1, if the name is "Untitled" followed by some digits
(now I look at it, it may be more correct to use + rather than *, to
ensure you do get some digits). 

You should be using this factory method to create nodes in the parser;
at present, you implement node creation again in the parser.

> 
> Barry, If you remember I should you this function in factory when we met last
> time.
> 
> I will make sure the list is maintained in the Diagram as instance object 
> rather
> than static.

It already is; it just needs to be accessible by the registrar, which
means that your methods in there take a diagram as an additional
parameter (and you dispense with the list member of that class).

> 
> - Ravish.
> 
> Quoting Andrew John Hughes <address@hidden>:
> 
> > On Thu, 2005-03-31 at 14:09 +0100, Barry Norton wrote:
> > > OK, now I'm more than a little annoyed - I didn't explain why I wanted
> > > the behaviour I did, nor why I wanted the code in the place I did,
> > > because I thought it would be enough that I had stated it as a
> > > requirement.
> > > 
> > 
> > I agree; this seems to have dragged on far too long for what is
> > relatively minor behaviour.
> > 
> > > If I have to be completely explicit to have something done the way I
> > > want then here goes:
> > > 
> > > 1) I want the behaviour of auto-naming being based on a simple counter
> > > and increment (rather than a search for the lowest unused name) because
> > > this is part of interactive editing and so must be as quick as possible.
> > > 
> > 
> > I implemented this in the factory I mentioned in my reply to Ravish's
> > mail.  This was done several weeks ago.
> > 
> > > 2) I want the behaviour after re-loading a file to be the same (i.e.
> > > next highest name than maximal in diagram, rather than going back and
> > > 'filling in the blanks'), because a different behaviour after saving and
> > > reloading the file breaks the user expectation - i.e. consistency is
> > > important, speed is less so on loading a file (this is based on blocking
> > > IO on a disk read, so the additional time working out the largest
> > > 'unnamed' name is negligible)
> > > 
> > 
> > This is what I believe I implemented in the factory also; as I noted in
> > my previous reply, Ravish seems to have ignored this and implemented it
> > in a similar way in the parser, albeit without the counter.  That said,
> > the method may be buggy, as I was unable to test it at the time (the
> > parser being unwritten).
> > 
> > > 3) I want the functionality that achieves all this in the factory
> > > because otherwise it is going to be massively duplicated (a common
> > > method in the future abstract ProcessFactory superclass should take care
> > > of this, not several separate functions in the parsing functionality).
> > > 
> > 
> > Agreed.
> > 
> > > 4) I don't want a static list of node names because (as I've now shown,
> > > despite repeated claims that this wasn't possible) it is possible to
> > > have two diagrams open for editing in the same Eclipse instantiation and
> > > this prevents having two nodes/processes with the same name *in the
> > > different diagrams*!
> > > 
> > 
> > I implemented the current system (more a rather nasty hack) as a
> > demonstration more of what needed to be done, and because I couldn't
> > fathom how to get hold of the diagram from the appropriate context.  It
> > should be a simple matter to modify this to use a passed-in Diagram
> > instance rather than a static list, if someone with more knowledge of
> > the internals than I knows how to pass in such instances.  Using the
> > diagram as the list obviously solves the bug mentioned above.
> > 
> > > In future if you're not sure whether something I've gone into details on
> > > is a requirement or just a suggestion, please ask before assuming the
> > > latter.
> > > 
> > > Thanks to Ravish for implementing the functionality, once I explained
> > > the first two of these points, but if re-factoring is necessary to do it
> > > the way I said, please get that done immediately.
> > > 
> > > And please sort out that final point before I see the editor again...
> > > 
> > > Barry
> > > 
> > 
> > I believe these items are documented in the minutes, and thus there is
> > really no excuse for things having gone so awry.  We also have a bug and
> > task database for such matters, which may be useful for this (and which
> > no-one seems to be using).
> > 
> > https://savannah.nongnu.org/bugs/?group=cashew-s-editor
> > 
> > https://savannah.nongnu.org/task/?group=cashew-s-editor
> > > 
> > > 
> > > 
> > > 
> > > 
> > > > -----Original Message-----
> > > > From:
> > > address@hidden
> > > > [mailto:cashew-s-editor-patches-
> > > > address@hidden On Behalf Of Andrew John
> > > > Hughes
> > > > Sent: Thursday, March 31, 2005 1:19 PM
> > > > To: CASHeW-s Editor Patches
> > > > Subject: Re: [CASHeW-s-editor-patches] I am commiting changes to
> > > > supportconsistent behavour (starting from max always) of editor.......
> > > > 
> > > > On Thu, 2005-03-31 at 03:43 +0100, Ravish Bhagdev wrote:
> > > > > I am comming the attached patch to allow only nodes with maximum
> > > number
> > > > from the nodes always, as Barry suggested in last meeting.  Three
> > > model
> > > > files are modified as mentioned below (patch attached):
> > > > >
> > > > > Changelog:
> > > > >
> > > > > 2005-03-31  Ravish Bhagdev  address@hidden
> > > > >
> > > > > * src/nongnu/cashews/eclipse/model/Connection.java
> > > > > * src/nongnu/cashews/eclipse/model/DiagramParser.java
> > > > > * src/nongnu/cashews/eclipse/model/NodeRegistrar.java
> > > > >
> > > > > - getSource() method added in Connection class
> > > > > - Parser modified to remove a bug causing repetition of outgoing
> > > edges
> > > > in model.
> > > > > - Added private int max property in NodeRegistrar along with getter
> > > and
> > > > setter methods for the same.
> > > > >
> > > > > - Ravish.
> > > > > _______________________________________________
> > > > > CASHeW-s-editor-patches mailing list
> > > > > address@hidden
> > > > > http://lists.nongnu.org/mailman/listinfo/cashew-s-editor-patches
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > ------------------------------------------------------------------------
> > > > --------
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > CASHeW-s-editor-patches mailing list
> > > > > address@hidden
> > > > > http://lists.nongnu.org/mailman/listinfo/cashew-s-editor-patches
> > > > > _______________________________________________
> > > > > CASHeW-s-editor-patches mailing list
> > > > > address@hidden
> > > > > http://lists.nongnu.org/mailman/listinfo/cashew-s-editor-patches
> > > > 
> > > > I don't get why you create the Nodes manually in the Parser, rather
> > > than
> > > > using the factory, which already has a method for capturing the node
> > > > number.  It seems we have duplicate functionality here.
> > > > 
> > > > Also, I don't know what went wrong with the patch, but it seems to
> > > > replace the entire parser, even though not all of it has changed.
> > > > --
> > > > Andrew :-)
> > > > 
> > > > Please avoid sending me Microsoft Office (e.g. Word, PowerPoint)
> > > > attachments.
> > > > See http://www.fsf.org/philosophy/no-word-attachments.html
> > > > 
> > > > No software patents in Europe -- http://nosoftwarepatents.com
> > > > 
> > > > "Value your freedom, or you will lose it, teaches history.
> > > > `Don't bother us with politics' respond those who don't want to
> > > learn."
> > > > -- Richard Stallman
> > > > 
> > > > Escape the Java Trap with GNU Classpath!
> > > > http://www.gnu.org/philosophy/java-trap.html
> > > > public class gcj extends Freedom implements Java { ... }
> > > 
> > > 
> > > 
> > > 
> > > _______________________________________________
> > > CASHeW-s-editor-patches mailing list
> > > address@hidden
> > > http://lists.nongnu.org/mailman/listinfo/cashew-s-editor-patches
> > > 
> > 
> > Thanks,
> > -- 
> > Andrew :-)
> > 
> > Please avoid sending me Microsoft Office (e.g. Word, PowerPoint)
> > attachments.
> > See http://www.fsf.org/philosophy/no-word-attachments.html
> > 
> > No software patents in Europe -- http://nosoftwarepatents.com
> > 
> > "Value your freedom, or you will lose it, teaches history.
> > `Don't bother us with politics' respond those who don't want to learn."
> > -- Richard Stallman
> > 
> > Escape the Java Trap with GNU Classpath!
> > http://www.gnu.org/philosophy/java-trap.html
> > public class gcj extends Freedom implements Java { ... }
> > 
> > 
> 
> 
> 
-- 
Andrew :-)

Please avoid sending me Microsoft Office (e.g. Word, PowerPoint)
attachments.
See http://www.fsf.org/philosophy/no-word-attachments.html

No software patents in Europe -- http://nosoftwarepatents.com

"Value your freedom, or you will lose it, teaches history.
`Don't bother us with politics' respond those who don't want to learn."
-- Richard Stallman

Escape the Java Trap with GNU Classpath!
http://www.gnu.org/philosophy/java-trap.html
public class gcj extends Freedom implements Java { ... }

Attachment: signature.asc
Description: This is a digitally signed message part


reply via email to

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