New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complex changes and enhancements as described below #7

Merged
merged 19 commits into from Oct 28, 2015

Conversation

Projects
None yet
2 participants
@codemanyak
Collaborator

codemanyak commented Oct 28, 2015

The issues listed below (internally registered with numbers KGU#..) are addressed by the modifications.

  1. Homepage launch on Menu item Help > New versions (fix KGU#35 CONFLICTING -> solved in favour of fesch's modification)
    The HTML-formatted label in the OptionPane in Diagram.updateNSD() didn't work as hyperlink and none of the two buttons of the dialogbox had any distinguishable effect.
  2. The comment popup in Element.E_SHOWCOMMENTS mode had had two major flaws (KGU#25, KGU#1):
    a) comments of inner elements weren't popped up due to a missing recursive element detection;
    b) popped up comments used to be sticky on the top drawing level on the screen even if the cursor had left the diagram.
    Both malfunctions are fixed now:
    wrt a) by properly subclassing method Element.getElementByCoord(int,int) and by the way merging it with method Element.selectElementByCoord(int,int) into Element.getElementByCoord(int,int,boolean).
    wrt b) by switching off the visibility of the comment pop on MouseExit().
  3. Consistent unselection before NSD export and execution (fix KGU#41)
    Before exporting an NSD, a kind of unselecting was done by the respective Diagram methods. This had been done in an incomplete way, however, as it did reset the attribute "selected" of any Element, but lacked to empty the "selected" attributes of Diagram itself. This had often led to very inconvenient behaviour due to the hidden pseudo-selection still being delivered by Diagram.getSelection().
    This is now fixed by introduction of a new method Diagram.unselectAll() incorporating
    a) the selectElementByCoord(-1,-1) call,
    b) the clearing of attributes selected, selectedDown, selectedUp, selectedMoved,
    c) and the eventual redraw() call.
    Moreover, this is now done before NSD execution, too. (Where it had been missing, possibly leaving the former selected element marked besides the element under execution, which had been irritating at least.)
    However, it remains still possible to select an element during executing (with no functional effect except showing it as selected, so far), though. This is particularly useful for the breakpoint support.
  4. Colour setting noticed as NSD change (fix KGU#38).
    The setting of an Element's colour had not been noticed as a diagram change, so it used to be ignored on exiting or replacing an NSD file. This is now fixed.
  5. Localised texts for the "enlarge FALSE" option (fix KGU#37)
    The relatively new checkbox "enlarge FALSE" on the Preferences panel (altPadRight toggle) hadn't got translations so far. They are now provided in some of the language files (Other languages should follow but are beyond the expertise of the author.)
    Affected files: de.txt, en.txt, es.txt, ru.txt
  6. Execution code decomposed, revised and enhanced by a useful breakpoint mechanism (fixes KGU#9, KGU#20, KGU#43)
    a) KGU#9: Element highlighting on execution didn't work while the delay time had been set to zero, which is okay in run mode but is not in step mode or on pausing. This is now fixed.
    b) KGU#20: The variable display in the JTable widget of the Control window didn't show any changes, either, while delay time being zero, not even on pausing or in step mode. This is now fixed as well.
    c) KGU#43: I introduced the concept of breakpoints on arbitrary elements. There are three GUI hooks to control them: for selected Elements there are a breakpoint toggle item in the context menu and a checkbox in the InputBox, for wiping off the breakpoints all over the diagram there is an image button in the Turtle/interpret toolbar. The breakpoint enhancement involved a paradigm change with respect to the position of the delay within the instruction: Whereas until now the display represented the state AFTER having executed the current Element, it will now show the state immediately BEFORE the current instruction. For consistency reasons, this holds for breakpoints as well as for pausing and single-step mode. The way the set breakpoints are drawn - as a red horizontal bar beneath the top edge of the Element rectangle, should be intuitive enough to convey this paradigm. Anyway, this is quite common in usual IDEs.
    On this occasion the overloading of the flag 'Element.selected' was lifted. Now the selection for editing elements is totally separated from execution status flagging. The latter is now only be done via the flags 'waited' (as before) and 'executed' (newly introduced). I may also have changed the meaning of the waited status - it now remains sticky on all uncompleted superstructure Elements. I chose to melt all the somewhat inconsistent code sequences determining the fill colour, which used to be distributed over nearly a dozen draw methods, into a single method on the abstract base class Element with name getFillColor(). It establishes four universal highlighting levels for Elements, ranking as follows: waited, executed, selected, none. This priority is more or less what I extracted from the legacy code - it might of course be changed but seems rather plausible.
    Method Executor.Step() has been split into subroutines, radically simplified and homogenized. This way, the fix for issues KGU#44 and KGU#47 (see below) became quite obvious and simple.
  7. Missing execution implementation for FOREVER loops (fix KGU#44)
    Apparently, there has never been an execution support for FOREVER loops - this might simply have been forgotten, I suppose. I fixed it by means of a common parameterised private method Executor.stepWhile(Element, boolean) as a variant of the While loop.
  8. Missing execution implementation for PARALLEL sections (fix KGU#47)
    For the Parallel section there has never been any execution strategy, either. At least a randomly serialized execution had to be provided for sensible testing. This is what I did by means of method Executor.stepParallel(). (An improved version will be part of the next pull request ahead).
  9. Delay-Slider in Control window was too short to handle it sensibly (fix KGU#8)
    Hard-coded change in the layout file - possibly this better ought to be done in a Netbeans GUI design tool (the code I changed seems to be generated automatically)?
  10. Sensible title translation for the InputBox (fix KGU#42)
    The title of the Element editor showed either hard-coded English texts provided by the button callback functions (that distinguished between the types of edited Elements) or a translated type-unspecific text provided by a "title" entry in the respective language file.
    I introduced a hook in the LangDialog mechanism, looking for patterns like .class_specific... in the language file. If such a pattern is found, a new subclassable instance method setLangSpecific(StringList keys, String translated) is called, which may do object-specific applicability tests or further computations over the remaining key list combined with own information.
    Suitable configurations for InputBox are provided with de.txt, es.txt, ru.txt, and it.txt, whereas in en.txt only some of the enhancements were necessary (hard-coded messages fit in here).
  11. Spelling corrections in it.txt
    Some obviously mis-spelled words were corrected (e. g. "Cilc" -> "Ciclo", "Contenuro" -> "Contenuto")
  12. Drawing of comment bars unified on class Element.
    Identical or closely similar code passages concerning the drawing of the grey comment bar existed in all draw methods of the Element subclasses. These lines of code were extracted and put into a protected helper method on the abstract base class Element.
  13. Arranger didn't keep track of diagram replacements (KGU#48)
    The Arranger used to appear nearly useless, because it seemed only to allow the creation of new diagrams from scratch: As soon as the user loaded an existing nsd file into a dependent Structorizer Mainform, the Arranger Surface immediately lost track and did not show the loaded diagram nor any subsequent activities of that Mainform. It just still held the original diagram. This was at least annoying though it has of course been possible to drop existing diagram files into the Arranger window in order to work upon. But this feature wasn't that obvious. With the fixes, a Root object will inform the Arranger (i.e. all its registered Updaters) about its replacement within a sub-process Mainform.
  14. Diagram losses on closing Arranger window or resetting diagrams (KGU#49)
    When the Arranger window was closed, all the dependent Structorizer Mainforms closed synchronously without giving the user any chance to save recent changes to disk. Also, any changes done on a subsequently created or loaded diagram (cf 13/KGU#48) of a dependent Mainform got lost without warning as soon as the original Root still held by the Arranger Surface was double-clicked. Both is now fixed (i.e. due to the KGU#48 fix the latter case has become more or less impossible). In a loop all Structorizer instances with "dirty" diagrams will ask, whether the changes be saved. This popup dialog will now always show either the already registered file path or the diagram's name (being the proposal for the subsequently popped-up file selection dialog).
    NOTE: This latter change ought to be maintained on merging with a very close new modification by fesch.
  15. Some hidden preparations for the intended enhancement to allow subdiagram calls (KGU#2)
    I approached a little closer to my old aim of enabling calls to open subroutine diagrams from a currently executed program diagram. To accomplish this aim got into reach (my ideas have become much clearer now) but will still need some time.
  16. Uniform file basename proposal via method Root.getMethodName()
    Recently, there have been up to a dozen different file export methods (e.g. in Diagram, Generator etc.) where a proposed file name was concocted by the respective service class, though in more or less the same way. These code parts have been replaced by mere calls of Root.getMethodName(), which provides good enough a sound proposal.
  17. Indentation issues in Code Generators (KGU#51)
    In most code generators the indentation mechanism was defective or error-prone (Oberon e.g. duplicated the indentation each level, most others relied on a single-character indentation unit). Moreover, the base Generator class itself initiated generateCode() with a non-empty indentation (a tabulator), only to replace alle the indented characters afterwards! This involves the risk that all such characters would be replaced, even if they had nothing to do with indentation.) So, why not insert the final indentation units in the first place? All generators had to be touched to mend this. On this occasion the comment insertion interface was slightly revised: parameter list simplified and a block commment method added.
  18. Shell functions and for loops in BASHGenerator (KGU#53)
    Functions have not been translated sensibly into the shell syntax. Arguments in particular cannot be passed via the parenthesis, so the following translation was implemented:
    Structorizer: name(arg1, arg2, ... argn)
    --> Bash: name() { arg1=$1; arg2=$2; ...; argn=$n; ... }
    The translation of the for loop into a for-in loop didn't make sense. I produced a quick-and-dirty change according to a similar quick-and-dirty code from CGenerator. The aim is the new BASH For loop syntax like
    for (( var=sv ; var<=ew ; var++ ))
  19. Function header in PasGenerator
    The translation of function headers on code export to Pascal was slightly improved.
  20. Bugfix in PythonGenerator: The Repeat loop generator "ate" two lines of the loop body (KGU#54)
    It seems that the author got confused about occurring empty lines at the end of the loop. Those empty lines of code, however, obviously originated from several generateCode methods that each added one explicitly. The insertion of empty lines after an empty line is now disabled as well.

codemanyak added some commits Oct 26, 2015

Update Element.java
Methods selectElementByCoord(x,y) and getElementByCoord() merged
Comment drawing centralized and breakpoint mechanism prepared
Execution state separated from selected state
Update Alternative.java
Method selectElementByCoord(int,int) replaced by getElementByCoord(int,int,boolean)
Comment drawing centralized and breakpoint mechanism prepared
Update Call.java
Methods selectElementByCoord(x,y) and getElementByCoord() merged
Comment drawing centralized and breakpoint mechanism prepared
Execution state separated from selected state
Erroneous commit overridden
Compare this with the original version, not with the most recent one.
Fixes by 2015-10-15
Method selectElementByCoord(int,int) replaced by getElementByCoord(int,int,boolean)
Comment drawing centralized and breakpoint mechanism prepared
@codemanyak

This comment has been minimized.

Show comment
Hide comment
@codemanyak

codemanyak Oct 26, 2015

Owner

This commit was a pure mistake. Accidently, the entire code of the Call.java file had been overwritten by a different class. Should have to be undone.

Owner

codemanyak commented on 937625c Oct 26, 2015

This commit was a pure mistake. Accidently, the entire code of the Call.java file had been overwritten by a different class. Should have to be undone.

codemanyak added some commits Oct 26, 2015

Fixes by 2015-10-15
Just the file comment corrected.
Fixes by 2015-10-15
Method selectElementByCoord(int,int) replaced by getElementByCoord(int,int,boolean)
Comment drawing centralized and breakpoint mechanism prepared.
Fixes by 2015-10-15
Method selectElementByCoord(int,int) replaced by getElementByCoord(int,int,boolean)
Breakpoint support prepared
Fixes by 2015-10-15
Highlighting unified
Breakpoint support added
Selection mechanism reorganised
Fixes by 2015-10-15
Comment drawing centralized and breakpoint mechanism prepared
Fixes by 2015-10-15
Method selectElementByCoord(int,int) replaced by getElementByCoord(int,int,boolean)
Comment drawing centralized and breakpoint mechanism prepared
Fixes by 2015-10-15
Method selectElementByCoord(int,int) replaced by getElementByCoord(int,int,boolean)
Breakpoint support added
Update Root.java
Methods selectByCoords() and getByCoords() merged
Highlighting / colouring revised
Breakpoint support added
Fixes by 2015-10-15
Method selectElementByCoord(int,int) replaced by getElementByCoord(int,int,true)
Comment drawing centralized and breakpoint mechanism prepared.
Fixes by 2015-10-15
Method selectElementByCoord(int,int) replaced by getElementByCoord(int,int,boolean)
Breakpoint support prepared
Code changes and additions, chiefly for breakpoint support, combined …
…with several bugfixes (Fixes by 2015-10-15):

1. Homepage launch on Menu item Help > New versions (fix KGU#35)
2. Comment popup flaws in Element.E_SHOWCOMMENTS mode mended (KGU#25, KGU#1)
3. Consistent unselection before NSD export and execution (fix KGU#41)
4. Colour setting noticed as NSD change (fix KGU#38)
5. Localised texts for the "enlarge FALSE" option (fix KGU#37)
6. Execution code decomposed, revised and enhanced by a	useful breakpoint mechanism (fixes KGU#9, KGU#20, KGU#43)
7. Missing execution implementation for FOREVER loops (fix KGU#44)
8. Missing execution implementation for PARALLEL sections (fix KGU#47)
9. Delay-Slider in Control window was too short to handle it sensibly (fix KGU#8)
10. Sensible title translation for the InputBox (fix KGU#42)
11. Spelling corrections in it.txt
12. Drawing of comment bars unified on class Element.
Transient WindowsListener added enabling Surface to have dirty diagra…
…ms saved before exit (fix KGU#49)

Several enhancements to improve Arranger usability (KGU#48, KGU#49)
Fixes as delivered by 2015-10-19
Comprising following bugfixes and enhancements:
- Arranger didn't keep track of diagram replacements (KGU#48)
- Diagram losses on closing Arranger window or resetting diagrams (KGU#49)
- Preparations for the intended enhancement of callable subdiagrams (KGU#2)
- Uniform file basename proposal via method Root.getMethodName()
- Indentation issues in Code Generators (KGU#51)
- Shell functions and FOR loops in BASHGenerator (KGU#53)
- Function header in PasGenerator
- PythonGenerator "ate" two lines of any Repeat loop body (KGU#54)
- fullText methods on all Element classes reorganised.

KNOWN ISSUES:
- If strings configured in the Parser Preferences contain regex-active characters (e.g. '?', '(', ')' etc.) then the line conversion for code export oder Executor might fail now - to fixed as soon as possible.
@fesch

This comment has been minimized.

Show comment
Hide comment
@fesch

fesch Oct 28, 2015

Owner

WoW!

Owner

fesch commented Oct 28, 2015

WoW!

@fesch fesch merged commit 70925c9 into fesch:master Oct 28, 2015

@fesch

This comment has been minimized.

Show comment
Hide comment
@fesch

fesch Oct 28, 2015

Owner

Okay, I hope I got the merge correctly done ...

Owner

fesch commented Oct 28, 2015

Okay, I hope I got the merge correctly done ...

@fesch

This comment has been minimized.

Show comment
Hide comment
@fesch

fesch Oct 28, 2015

Owner

Maybe you could also write a "shorter" description about the changes in the changelog.txt file located in the "gui" package. The version can also be increased in the "Element" class ...

Owner

fesch commented Oct 28, 2015

Maybe you could also write a "shorter" description about the changes in the changelog.txt file located in the "gui" package. The version can also be increased in the "Element" class ...

@codemanyak

This comment has been minimized.

Show comment
Hide comment
@codemanyak

codemanyak Oct 29, 2015

Collaborator

Hi Bob,

of course I'm willing to do so, only it will take one more day (till
Friday afternoon) to find the time to accomplish that if this os okay.

Thanks and regards
Kay

Am 28.10.2015 um 19:31 schrieb Bob Fisch:

Maybe you could also write a "shorter" description about the changes in the changelog.txt file located in the "gui" package. The version can also be increased in the "Element" class ...

Collaborator

codemanyak commented Oct 29, 2015

Hi Bob,

of course I'm willing to do so, only it will take one more day (till
Friday afternoon) to find the time to accomplish that if this os okay.

Thanks and regards
Kay

Am 28.10.2015 um 19:31 schrieb Bob Fisch:

Maybe you could also write a "shorter" description about the changes in the changelog.txt file located in the "gui" package. The version can also be increased in the "Element" class ...

@codemanyak

This comment has been minimized.

Show comment
Hide comment
@codemanyak

codemanyak Oct 30, 2015

Collaborator

Hi Bob,

the merging seems okay but there is something odd about your change #6
to the windowClosing event handler in Mainform.create(). I haven't fully
understood what you intended with the change, but you added a
system.exit(0); - and this will immediately kill an owning Arranger and
all its threads (including dependent other Structorizer Mainfraims
without any warning or chance to store unsaved changes!).

So please think twice about this fix (#6). Seems it causes more trouble
than it mends.

Regards
Kay

Collaborator

codemanyak commented Oct 30, 2015

Hi Bob,

the merging seems okay but there is something odd about your change #6
to the windowClosing event handler in Mainform.create(). I haven't fully
understood what you intended with the change, but you added a
system.exit(0); - and this will immediately kill an owning Arranger and
all its threads (including dependent other Structorizer Mainfraims
without any warning or chance to store unsaved changes!).

So please think twice about this fix (#6). Seems it causes more trouble
than it mends.

Regards
Kay

@fesch

This comment has been minimized.

Show comment
Hide comment
@fesch

fesch Oct 30, 2015

Owner

Yes, you are right, as that command not only kills the actual window. I should use close() instead ...

Owner

fesch commented Oct 30, 2015

Yes, you are right, as that command not only kills the actual window. I should use close() instead ...

fesch pushed a commit that referenced this pull request Oct 30, 2015

@codemanyak

This comment has been minimized.

Show comment
Hide comment
@codemanyak

codemanyak Oct 30, 2015

Collaborator

Hi Bob,

it's still handled wrong, I'm afraid.
Let's first make sure what the intensions are.
If I understand your addressed issue well then you want the Mainfraim
regularly closed when in the JOptionDialog "Do you want to save the
current NSD-File?" either "yes" or "no" is pressed but it is to be
maintained if the JOptionDialog is just closed without pressing any of
the buttons (let's say "cancelled").

If so then in Diagramm.saveNSD() all depends on the value of res: The
return value must distinguish between res = -1 (JOptionDialog cancelled
in order to avoid closing the Mainform) and all other cases (res = 0,
res = 1, or not asked at all).
Intuitively I propose result value false if the JOptionDialog was
cancelled. The intelligible way to code this would look like in the
appended code snippet I think. Something suggests me that this was what
you intended but failed to achieve.

Obviously the change in Mainform was totally wrong because you disabled
closing the window in ANY case by inserting line
setDefaultCloseOperation(JFrame.DO_NOTHING_ON_CLOSE);
before establishing the windowClosing handler. So it's understandable
that you shot the Mainfraim down with a gun, since nothing happened at all,
whatever the saveNSD method might have returned. Particularly, the if
condition has to be negated if the above suggestion for the return value
semantics is to hold.

The trouble is, however: If you once cancelled the close action then how
and when to reestablish the EXIT_ON_CLOSE behaviour, which of course
must be the standard behaviour?
The most sensible way to achieve this is employing a complete
alternative in the windowClosing handler as done the appended code snippet.

I just tested it. I propose to commit it to my branch and send you a
pull request (together with the changelog.txt changes).

Regards Kay

Am 30.10.2015 um 06:44 schrieb Bob Fisch:

Yes, you are right, as that command not only kills the actual window. I should use close() instead ...

Collaborator

codemanyak commented Oct 30, 2015

Hi Bob,

it's still handled wrong, I'm afraid.
Let's first make sure what the intensions are.
If I understand your addressed issue well then you want the Mainfraim
regularly closed when in the JOptionDialog "Do you want to save the
current NSD-File?" either "yes" or "no" is pressed but it is to be
maintained if the JOptionDialog is just closed without pressing any of
the buttons (let's say "cancelled").

If so then in Diagramm.saveNSD() all depends on the value of res: The
return value must distinguish between res = -1 (JOptionDialog cancelled
in order to avoid closing the Mainform) and all other cases (res = 0,
res = 1, or not asked at all).
Intuitively I propose result value false if the JOptionDialog was
cancelled. The intelligible way to code this would look like in the
appended code snippet I think. Something suggests me that this was what
you intended but failed to achieve.

Obviously the change in Mainform was totally wrong because you disabled
closing the window in ANY case by inserting line
setDefaultCloseOperation(JFrame.DO_NOTHING_ON_CLOSE);
before establishing the windowClosing handler. So it's understandable
that you shot the Mainfraim down with a gun, since nothing happened at all,
whatever the saveNSD method might have returned. Particularly, the if
condition has to be negated if the above suggestion for the return value
semantics is to hold.

The trouble is, however: If you once cancelled the close action then how
and when to reestablish the EXIT_ON_CLOSE behaviour, which of course
must be the standard behaviour?
The most sensible way to achieve this is employing a complete
alternative in the windowClosing handler as done the appended code snippet.

I just tested it. I propose to commit it to my branch and send you a
pull request (together with the changelog.txt changes).

Regards Kay

Am 30.10.2015 um 06:44 schrieb Bob Fisch:

Yes, you are right, as that command not only kills the actual window. I should use close() instead ...

codemanyak added a commit that referenced this pull request Dec 21, 2015

fesch pushed a commit that referenced this pull request Oct 29, 2017

@fesch fesch referenced this pull request Jun 10, 2018

Closed

Weird bug #536

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment