KIELER Meeting 02/09/2011

• Moderator: msp
• Protocol: cds
• Attendees:
  • bdu
  • cds
  • chsch
  • ckru
  • jes
  • mri
  • msp
  • pkl
  • rvh
  • uru
• Start: 10:00h
• End: 11:05

Agenda

• Layout Annotations (msp)
• Checkstyle (cds)

Layout Annotations (msp)

• The sync charts textual view now supports annotations saved in the model:

@AnnotationID "AnnotationValue"

• These annotations can be used to set layout options
• Naming of annotations
  • Annotations are represented in the text view by either their complete ID (de.cau.cs.kieler.layout.options.layoutDirection) or by the last part of their ID (layoutDirection).
  • Thus, internal names are now exposed to the user. We should make sure that the names make sense. IDs that don't make sense should be changed.
    • Pro: The IDs are better hints as to what the annotation does.
    • Contra: Models referring to the old IDs lose the settings of options whose IDs have changed.
    • Decision: #1458 Still, pro outweighs contra, so do that!
  • General strategy for the renaming:
    • IDs of annotations should be consistent with the names used for them in the UI. (Example: "Layout Algorithm" should map to the ID layoutAlgorithm, not layoutHint)
      • Drop spaces
      • Drop hyphens
      • Camel case
  • The renaming also applies to layout algorithms, whose IDs are exposed in the value of the layoutAlgorithm annotation.
    • The same general strategy for renaming those applies here.
    • In addition, the word "Layout" should be dropped in the IDs. ("Hierarchical Layout" becomes hierarchical instead of hierarchicalLayout)
• Recursive annotations
  • Annotations could express recursion. An example is the layoutHV annotation, which goes down the hierarchy of a syncchart and alternatingly applies a horizontal and vertical layout to the different states it finds.
  • rvh would like it if any annotation could be applied recursively by appending another keyword. (this would remove the need for the layoutHV annotation) Possibilities include:

@layoutDirection "Recursive: Right Down"
@layoutDirection rec "Right Down"

• msp says that the topic of recursive annotations is complex and would make a good thesis in its own right.
• #1459 Thus, for now rvh proposes the compromise of having certain special recursive annotations, as such:

@layoutDirectionRecursive "RightDown"
@layoutDirectionRecursive "Right Down"

• Problems found during the presentation:
  • #1460 The Remove all Layout Options command in the Layout view doesn't correctly update the value shown in the table.
  • #1461 The Layout algorithm option in the Layout view opens the Layout algorithm selection dialog upon the first click on the option. Better: Display a standard "..." button on the first click, open the dialog when the user double-clicks the option or clicks the "..." button.
  • #1462 When a state in a sync chart only has one region, that region is omitted in the text view, even if the region has annotations. But in that case, the region should not be omitted so that its annotations can be displayed and are not attached to the region's state.
  • #1463 Annotations should be case insensitive.
  • #1464 The text view should support content assist for annotations, possibly with explanations of what an annotation does and what parameters it expects. (as in Eclipse, where content assist displays the Javadoc information for methods and parameters)
  • #1465 Annotations currently must have a value for the text view to be content. But there are annotations without a value. Don't highlight those as errors.
  • #1466 In the text view, annotations are often appended to an existing line. Instead, they should always be displayed on their own line.
  • #1467 Values of annotations need not be strings. They can quite possibly also be numbers, but the text view marks those as errors. Example:

@minSpacing "100.5"      // No error, but the number must be written as String
@minSpacing 100.5        // Error where there shouldn't be one

Checkstyle (cds)

• In the last meeting, haf complained about Checkstyle's Hidden field warnings.
• cds has changed the Checkstyle configuration file accordingly. Now, it behaves as follows:

public class Foo {
    private int bar = 0;

    public Foo(final int bar) {                // No warning
        this.bar = bar;

        int bar = 3;                           // Warning
    }

    public void setBar(final int bar) {        // No warning
        this.bar = bar;

        int bar = 3;                           // Warning
    }
    
    public void anotherMethod(final int bar) { // Warning
        int bar = 3;                           // Warning
    }
}

• Warnings on property files (build.properties, plugin.properties, ...) cannot be disabled for everyone globally. The way to go here is to disable Checkstyle for property files in the project properties.
• A short religious discussion ensued about other warnings and the Eclipse Java formatter. In particular, mri doesn't like the convention of lines being up to 105 characters long. Everyone else does.