[JaMoPPC] option to disable layout information recording

[jjohannes]

A command line argument to set the IJavaOptions.DISABLE_LAYOUT_INFORMATION_RECORDING and IJavaOptions.DISABLE_LOCATION_MAP (I think the latter can always be set to improve memory usage).

[tsdh]

Here's a patch:

From 445f863fb1e5cba40ba83bed159ff4e782d471a7 Mon Sep 17 00:00:00 2001
From: Tassilo Horn <tsdh@gnu.org>
Date: Wed, 29 Aug 2012 09:51:15 +0200
Subject: [PATCH] Allow for setting the IJavaOptions

  DISABLE_LAYOUT_INFORMATION_RECORDING and
  DISABLE_LOCATION_MAP

via system properties.
---
 .../org/emftext/language/java/jamoppc/JaMoPPC.java | 30 +++++++++++++++++++---
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/Utils/org.emftext.language.java.jamoppc/src/org/emftext/language/java/jamoppc/JaMoPPC.java b/Utils/org.emftext.language.java.jamoppc/src/org/emftext/language/java/jamoppc/JaMoPPC.java
index 0c81092..7d0ec9f 100644
--- a/Utils/org.emftext.language.java.jamoppc/src/org/emftext/language/java/jamoppc/JaMoPPC.java
+++ b/Utils/org.emftext.language.java.jamoppc/src/org/emftext/language/java/jamoppc/JaMoPPC.java
@@ -41,11 +41,24 @@ import org.emftext.commons.layout.LayoutPackage;
 import org.emftext.language.java.JavaClasspath;
 import org.emftext.language.java.JavaPackage;
 import org.emftext.language.java.resource.JavaSourceOrClassFileResourceFactoryImpl;
+import org.emftext.language.java.resource.java.IJavaOptions;

 public class JaMoPPC {

    protected static final ResourceSet rs = new ResourceSetImpl();

+   static {
+       Map<Object, Object> options = rs.getLoadOptions();
+       if (Boolean.valueOf(System.getProperty(
+               "JAMOPP_DISABLE_LAYOUT_INFORMATION_RECORDING", "false"))) {
+           options.put(IJavaOptions.DISABLE_LAYOUT_INFORMATION_RECORDING, true);
+       }
+       if (Boolean.valueOf(System.getProperty("JAMOPP_DISABLE_LOCATION_MAP",
+               "false"))) {
+           options.put(IJavaOptions.DISABLE_LOCATION_MAP, true);
+       }
+   }
+
    public static void main(String[] args) throws IOException {
        if (args.length < 2) {
            printUsageAndExit();
@@ -177,6 +190,16 @@ public class JaMoPPC {
        System.out.println();
        System.out
                .println("In the latter case, the second parameter has to end in \".xmi\".");
+       System.out.println();
+       System.out
+               .println("The creation of layout information can be suppressed using the");
+       System.out
+               .println("JAMOPP_DISABLE_LAYOUT_INFORMATION_RECORDING system property.");
+       System.out
+               .println("Addinionally, the creation of a location map can be suppressed with");
+       System.out
+               .println("the property JAMOPP_DISABLE_LOCATION_MAP.  Note that disabling the");
+       System.out.println("latter might cause increased memory usage.");
        System.exit(1);
    }

@@ -199,11 +222,10 @@ public class JaMoPPC {
                JavaPackage.eINSTANCE.getESubpackages());

        javaEcoreResource.save(null);
-           
+
        URI layoutEcoreURI = outUri.appendSegment("layout.ecore");
        Resource layoutEcoreResource = rs.createResource(layoutEcoreURI);
-       layoutEcoreResource.getContents().add(
-               LayoutPackage.eINSTANCE);
+       layoutEcoreResource.getContents().add(LayoutPackage.eINSTANCE);

        layoutEcoreResource.save(null);
    }
@@ -251,7 +273,7 @@ public class JaMoPPC {
        int eobjectCnt = 0;
        for (EObject next : eobjects) {
            eobjectCnt++;
-           if (eobjectCnt % 1000 == 0) {
+           if ((eobjectCnt % 1000) == 0) {
                System.out.println(eobjectCnt + "/" + eobjects.size()
                        + " done: Resolved " + resolved + " crossrefs, "
                        + notResolved + " crossrefs could not be resolved.");
-- 
1.7.12

[jjohannes]

Thanks. I would prefer to use a command line argument instead of the using system properties.

[tsdh]

I took the properties approach in order not to add some new dependency or reimplement a CLI parser on my own. But of course I can either

1) Add a proper CLI, e.g. using Apache Commons CLI (or whatever you prefer) 2) Add a poor-man's CLI, e.g. options have to come first (no dependencies)

[jjohannes]

I would only introduce one option for now "--discard-layout" (?) for DISABLE_LAYOUT_INFORMATION_RECORDING.

I would always set JAMOPP_DISABLE_LOCATION_MAP since this only influences in memory information (which is needed by the generated editor for instance). Since this information is never used in jamoppc, I do not see a point in enabling this option.

Since there is only one option now I would go with the poor-man's implementation but we should think about using Apache Commons CLI in the future if we further extend jamoppc.

[tsdh]

Ok, here's a new patch.

From dc50cb8d33ee9939c6e1dd6179528c28a94281d8 Mon Sep 17 00:00:00 2001
From: Tassilo Horn <tsdh@gnu.org>
Date: Thu, 30 Aug 2012 11:21:48 +0200
Subject: [PATCH] Add --disable-layout option.

---
 .../src/org/emftext/language/java/jamoppc/JaMoPPC.java   | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/Utils/org.emftext.language.java.jamoppc/src/org/emftext/language/java/jamoppc/JaMoPPC.java b/Utils/org.emftext.language.java.jamoppc/src/org/emftext/language/java/jamoppc/JaMoPPC.java
index 0c81092..66c66f9 100644
--- a/Utils/org.emftext.language.java.jamoppc/src/org/emftext/language/java/jamoppc/JaMoPPC.java
+++ b/Utils/org.emftext.language.java.jamoppc/src/org/emftext/language/java/jamoppc/JaMoPPC.java
@@ -18,6 +18,7 @@ package org.emftext.language.java.jamoppc;
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -41,12 +42,18 @@ import org.emftext.commons.layout.LayoutPackage;
 import org.emftext.language.java.JavaClasspath;
 import org.emftext.language.java.JavaPackage;
 import org.emftext.language.java.resource.JavaSourceOrClassFileResourceFactoryImpl;
+import org.emftext.language.java.resource.java.IJavaOptions;

 public class JaMoPPC {

    protected static final ResourceSet rs = new ResourceSetImpl();

    public static void main(String[] args) throws IOException {
+       if ((args.length >= 1) && args[0].equals("--disable-layout")) {
+           rs.getLoadOptions().put(IJavaOptions.DISABLE_LAYOUT_INFORMATION_RECORDING, true);
+           args = Arrays.copyOfRange(args, 1, args.length);
+       }
+       
        if (args.length < 2) {
            printUsageAndExit();
        }
@@ -166,17 +173,20 @@ public class JaMoPPC {
                        + "parsed compilation unit in the target folder, use:");
        System.out.println();
        System.out
-               .println("  jamoppc <source folder path> <target folder path> <jar file paths>*");
+               .println("  jamoppc [--disable-layout] <source folder path> <target folder path> <jar file paths>*");
        System.out.println();
        System.out
                .println("To parse all files in a source folder and produce one XMI file\n"
                        + "with the complete syntax graph, use:");
        System.out.println();
        System.out
-               .println("  jamoppc <source folder path> <target XMI file> <jar file paths>*");
+               .println("  jamoppc [--disable-layout] <source folder path> <target XMI file> <jar file paths>*");
        System.out.println();
        System.out
                .println("In the latter case, the second parameter has to end in \".xmi\".");
+       System.out.println();
+       System.out.println("The --disable-layout option disables the generation of layout information.");
+       System.out.println("If given, it must be the first argument given to JaMoPPC.");
        System.exit(1);
    }

@@ -251,7 +261,7 @@ public class JaMoPPC {
        int eobjectCnt = 0;
        for (EObject next : eobjects) {
            eobjectCnt++;
-           if (eobjectCnt % 1000 == 0) {
+           if ((eobjectCnt % 1000) == 0) {
                System.out.println(eobjectCnt + "/" + eobjects.size()
                        + " done: Resolved " + resolved + " crossrefs, "
                        + notResolved + " crossrefs could not be resolved.");
-- 
1.7.12

Could you please also regenerate the jar in the repository if the patch is ok?

[jjohannes]

I have applied the patch and added the DISABLE_LOCATION_MAP option. Please re-open the issue if you encounter problems.

[tsdh]

Seems to work fine, thanks.

BTW, it's generally better maintenance style to git apply contributed patches and add tweaks with another commit (or simply ask the contributor to tweak). That way, the contributor has the fame of appearing in the commit history, and you as a maintainer gets a list of contributors for free using git log | grep "Author:" | sort | uniq.

[jjohannes]

Thanks for testing! Sorry for applying the patch 'manually'. I used Eclipse>Team>Patch as I am used to, but this does not seem to redirect to 'git apply' in any way. And of course I should have splittet this into two commits anyhow.

I will give my best to improve in the future. :)

Anyway, maybe it's even easier if we do this via pull-requests in the future? Then it will be even more obvious and visible who contributed what.

Thanks again for contributing!

[tsdh]

Ah, you strange Eclipse "klicki-bunti" guys. ;-) But yes, sure, the next time I'll fork and supply a pull request.