FWD » Core Development » Base Language

• :L - Left Justified: Pads the string with spaces on the right to reach a specified length, aligning it to the left..
• :R - Right Justified: Pads the string with spaces on the left to reach a specified length, aligning it to the right..
• :C - Centered: Pads the string with spaces on both sides to center it within the specified length.
• :T - Trimmed: Removes leading and trailing blanks from the string, but still reserves space for the specified length when used in display or storage.

These options require a number after them (e.g. R10, L10) to specify the maximum length.

Constantin, Greg or anyone else can correct me if I'm wrong on this. There is also a :U option that is used to indicate that the string should not be translated but I don't think that this is needed. These options are disregarded at conversion time.

See the documentation for these options here

We parse those and already handle the :U case. Please show a 4GL testcase that demonstrates alignment/trimming behavior that is missing.

The code I was working on resembles the following:

DEFINE TEMP-TABLE tt NO-UNDO
    FIELD a AS CHARACTER.

CREATE tt.
tt.a = "characters":L30.

DEF VAR res AS LONGCHAR NO-UNDO.
TEMP-TABLE tt:HANDLE:WRITE-JSON(STRING(Progress.Reflect.DataType:LONGCHAR), res, TRUE).
COPY-LOB FROM res TO FILE "output.json".

Generates the following file:

{"tt": [
  {
    "a": "characters                    " 
  }
]}

Although probably the following instructions would be a better testcase as they don't behave the same:

DISPLAY "ThisIs a text.":R40.
DISPLAY "ThisIs another text.":C50.
DISPLAY "        This must be trimmed          ":T20.

Although probably the following instructions would be a better testcase as they don't behave the same:

I agree the display testcase is simpler than the tmep-table and write-json. They don't behave the same as compared to what?

The testcase should encode the output in a way that makes the behavior clear to the reader. Does the behavior require something like DISPLAY or can it be seen by just assigning the string literal and then writing code to look at or compare the contents to a hard coded string literal?

Yes, I understand that FWD currently ignores these. I want details on the OE behavior. Is it possible to write something like this (a testcase that doesn't rely upon scanning output to detect the differences):

def var ch1 as char.
ch1 = "XX":C10.
if ch1 ne "    XX    " then
do:
   ...do something to report the failure...
end.

The if could be switched to an assert to make this into an ABLUnit test. My point here: does this work in OE or is the behavior somehow related to formatted output?

Constantin Asofiei wrote:

Paul, try simple statements like message length("A":L20). or message length(" This must be trimmed ":T20).. DISPLAY has other implications on the value.

For example:

message length("A":L20). //ABL: 20 | FWD: 1
message length("       A B C      ":T10). //ABL: 5 | FWD: 18

The converted code is:

@LegacySignature(type = Type.MAIN, name = "start.p")
   public void execute()
   {
      externalProcedure(Start.this, new Block((Body) () -> 
      {
         message(length(new character("A")));
         message(length(new character("       A B C      ")));
      }));
   }

Greg Shah wrote:

Yes, I understand that FWD currently ignores these. I want details on the OE behavior. Is it possible to write something like this (a testcase that doesn't rely upon scanning output to detect the differences):

Oh I understand now. I think that for all of them we can use the LENGTH function and in combination with that, the MATCHES and/or the BEGINS operator for the :R and :L options.

What about something as simple as this:

   @Test.
   method public void testC10():
      Assert:IsTrue("XX":C10 eq "    XX    ").
   end.

Does this work in OE?

If this works, then these string options are just a "shorthand" for encoding string literals. We can simply do that translation at conversion time and leave the final string in the Java source code. There would be no reason to have any runtime implementation.

Greg, isn't 4GL right-trimming when it does comparisons?

I think we may need a 'char by char' comparison.

Constantin Asofiei wrote:

Greg, isn't 4GL right-trimming when it does comparisons?

I think we may need a 'char by char' comparison.

Yes, fair point. We can write a method public logical char-by-char-comparison(character one, character two) method and use that in the assert. Better yet, put that code in our helper support library so it can be shared by other tests.

I don't think that will work, as we will not be able to test in 4GL. But I think we can use memptr like this:

def var m1 as memptr.
set-size(m1) = 11.
put-string(m1, 1) = "t":10.
message get-size(m1) get-byte(m1, 2).

I don't think that will work, as we will not be able to test in 4GL.

I'm talking about the 4GL helper code in the support/ directory. This will work fine in both OE and FWD.

Greg, yes, that makes sense, I was reading the method as a Java method.

Paul: please use memptr to check char-by-char.

I expect the fix for the issue to be written in rules/convert/literals.rules.

Isn't character.progressToJavaString the place where we 'prepare' the 4GL string for Java?

It is where we convert the contents. Processing the options is better done in TRPL where we have access to the options in the AST and are going to emit the result.

Ok, I committed rev. 1789 on the branch of the testcases project class. The test class is tests/base_language/base_data_types/CharsWithOptions.cls

Code Review Task Branch 10290a Revision 16055

1. Why is the code inserted at the top of literals.rules instead of in this section (line 1200) which does the output for strings:

            <rule>type == prog.string and !suppress
               <variable name="strLiteral" type="java.lang.String"/>
               <rule>isNote("convertToRegExp")
                  <!-- this literal is a Progress pattern for MATCH -->
                  <action>strLiteral = ecw.convertToRegEx(text)</action>

                  <!-- otherwise convert from Progress directly to Java literal -->
                  <action on="false">strLiteral = ecw.progressToJavaString(text)</action>
                  <rule on="false">upPath(this, prog.statement, prog.kw_case, prog.kw_when, prog.expression)
                     <action>strLiteral = trimTrailing(strLiteral)</action>

                     <rule>copy.parent.parent.parent.getAnnotation("caseInsensitive")
                        <action>strLiteral = ecw.toUpperCase(strLiteral)</action>
                     </rule>
                  </rule>
               </rule>

               <action>ref = createPeerAst(java.string, strLiteral, getAst(parentid))</action>
               <action>execLib("wrapI18nLiteral", this, ref, false)</action>
            </rule>

2. I would prefer if processStringLiteral() returned the updated string or null if no changes were made. We don't want to edit the AST itself as part of this processing. We can simply include the changes in the output for Java.

3. Shouldn't the literals code use ecw.processStringLiteral(copy) instead of ecw.processStringLiteral(this)? Otherwise it won't pick up any changes that have otherwise been made to the string content.

4. Please remove the printfln output from the literals rules.

5. Please rename processStringLiteral() to processStringOptions()

6. The processing is hard coded to double quoted strings, it should handle both single and double quoted strings. Make sure that you have testcases included for these.

7. The code does not handle the full range of string options possibilities:

         ':'
         (
            options { generateAmbigWarnings = false; }:
               ('r' | 'l' | 'c' | 't') ('u' | 'x') (DIGIT)*
            |  ('r' | 'l' | 'c' | 't') (DIGIT)+ ('u' | 'x')?
            |  ('r' | 'l' | 'c' | 't')
            |  ('u' | 'x') ('r' | 'l' | 'c' | 't') (DIGIT)*
            |  ('u' | 'x') (DIGIT)+ ('r' | 'l' | 'c' | 't')?
            |  ('u' | 'x')
            | (DIGIT)+
              (
                   ( ('r' | 'l' | 'c' | 't') ('u' | 'x')? )
                 | ( ('u' | 'x') ('r' | 'l' | 'c' | 't')? )
              )?

Note how the order of these options is very flexible. The digits don't have to be at the end, there can be a U or X present at the same time as one of R, L, C, T. the R, L, C, T can be AFTER the digits and can even follow the U or X.

Please add tests for each of these cases. Check with the existing lexer tests to see if there is some useful code there.

Also note that the digits are optional when they appear at the end of the options.

We could make changes to the lexer to expose all of these as different tokens (it would avoid reparsing later) but the cost of doing that would be adding complications throughout our TRPL code. I prefer not to do that.

Paul, to summarize: create tests with all combination of string options, including 'numbers', and for numbers include negative values, zero, and less than the actual string constant's length.

I'm exploring regex rules, I think that this is the way to go because they are just so many combos.

Then we probably should store the parsed components in annotations on the AST node. Then the downstream processing will be trivial and the TRPL code can even pass the values in to the helper.

The "trick" is storing these changes without changing the overall structure of the tree. The annotations can't be stored in the lexer, only in the parser. This suggests that the lexer would split the string options out as tokens and the parser would store the contents in annotations, ensure that the text of the string is merged together and then drop those tokens so that there is only a STRING left behind.

Please test also if an option appears multiple times and also a number. And keep an eye if an option overrides another (if option X is present then option Y is ignored).

Greg Shah wrote:

Then we probably should store the parsed components in annotations on the AST node. Then the downstream processing will be trivial and the TRPL code can even pass the values in to the helper.

Ok, should I create another helper function in the ExpressionConversionWorker class that would return the options of the literal to store them in a annotation? Should I also put the annotation in literals.rules?

The "trick" is storing these changes without changing the overall structure of the tree. The annotations can't be stored in the lexer, only in the parser. This suggests that the lexer would split the string options out as tokens and the parser would store the contents in annotations, ensure that the text of the string is merged together and then drop those tokens so that there is only a STRING left behind.

I'm sorry I never had to work with this and probably don't understand it completely, are you suggesting to do the processing in the parser instead of in the literals.rules?

Another thing. I could not find what the X option does. I've made some experiments with it and it looks like it doesn't affect the output at all.

I have to look further at this. The change in the lexer and parser may be tricky.

The alternative is something like the regex approach you suggest but regexes are notoriously cryptic and hard to ensure correctness.

Paul Bodale wrote:

Another thing. I could not find what the X option does. I've made some experiments with it and it looks like it doesn't affect the output at all.

I suspect it is just a way to say "this string IS translatable" which is the same as the default when U is not present.

Greg Shah wrote:

I suspect it is just a way to say "this string IS translatable" which is the same as the default when U is not present.

That would mean that we should treat it the same as default, got it.

         ':'
         (
            options { generateAmbigWarnings = false; }:
               ('r' | 'l' | 'c' | 't') ('u' | 'x') (DIGIT)*
            |  ('r' | 'l' | 'c' | 't') (DIGIT)+ ('u' | 'x')?
            |  ('r' | 'l' | 'c' | 't')
            |  ('u' | 'x') ('r' | 'l' | 'c' | 't') (DIGIT)*
            |  ('u' | 'x') (DIGIT)+ ('r' | 'l' | 'c' | 't')?
            |  ('u' | 'x')
            | (DIGIT)+
              (
                   ( ('r' | 'l' | 'c' | 't') ('u' | 'x')? )
                 | ( ('u' | 'x') ('r' | 'l' | 'c' | 't')? )
              )?

• String rule1 = "([rRlLcCtT])([uUxX])?(\\d*)";
• String rule2 = "([uUxX])?([rRlLcCtT])(\\d*)";
• String rule3 = "(\\d+)([rRlLcCtT])([uUxX])?";
• String rule4 = "(\\d+)([uUxX])?([rRlLcCtT])";
• String rule5 = "([rRlLcCtT])(\\d+)([uUxX])?";
• String rule6 = "([uUxX])?(\\d+)([rRlLcCtT])";

The only unhanded case is ('u' | 'x') but this way we ensure that the all combination of the options are matched. We can disregard the U and X options as we don't need to handle them here. We can use the regex that was matched to easily extract the option and the number (if present) and we can mostly keep the current implementation.

OK, do that.

Code Review Task Branch 10290a Revision 16056

1. This code makes edits to this.text.

            <rule>type == prog.string and !suppress
               <variable name="processedLiteral" type="java.lang.String"/>
               <action>processedLiteral = ecw.processStringOptions(copy.text)</action>

               <rule>processedLiteral != null
                  <action>this.setText(processedLiteral)</action>
               </rule>

               <variable name="strLiteral" type="java.lang.String"/>
               <rule>isNote("convertToRegExp")
                  <!-- this literal is a Progress pattern for MATCH -->
                  <action>strLiteral = ecw.convertToRegEx(text)</action>
...

Doing that is not OK for multiple reasons.

• We NEVER edit this, edits only get made to copy. Any changes to this are not persisted so making those edits is confusing at best and often are just incorrect.
• In this scenario where we are generating Java output, it does not make sense to store the partially modified content of the string back into the AST. Doing so will corrupt the AST itself. It would no longer represent the original 4GL contents and it won't represent the Java contents, it just will be some intermediate state. That is not something that we want to persist.

Instead, please initialize the processedLiteral to text in the case where it is null AND then reference processedLiteral in the subsequent rules instead of text.

2. In ECW, please add javadoc to the rulePatterns member and de-indent the {} portion by 3 chars to align it with the preceding line.

3. Add javadoc for processStringOptions() to explain the return value.

4. The use of int colIndex = initialText.indexOf(stringDelimiter + ":"); is not guaranteed to reference string options. If the stringDelimiter is escaped inside the string and is followed by :, then this can match string contents.

5. The core logic of the method is hard to understand since it deeply encodes knowledge about specific elements of rulePatterns. It makes it impossible to understand without looking at the patterns themselves. Any edits to rulePatterns will also break this code badly which means the code is fragile. Moving away from the if ... else if ... else if .... else if ... else structure will help but only if we can move to a loop where the rulePatterns array is processed with generic code.

6. I think it may be possible to make the switch at the end have less duplicated code but at least that code is pretty understandable. If you can see a way to reduce that code it would be nice.

Greg Shah wrote:

The only thing is that for (Pattern pattern : rulePatterns) should be changed to explicitly iterate the array with an int subscript. We've found that the "for each" in Java is slower than explicitly iterating. Since this will be processed for every string that has options in the project, on big projects this may actually make a difference.

I committed rev. 16059 that includes this change and also the fix for a problem I found while converting 2 customer applications. If the literal was only ":" then the conversion would fail.

I ran the conversion for the 2 projects again and it succeeded. The only differences are related to some literals that were using the :T option which was disregarded. The correct form of this literals is reflected in the converted code. If you would like more details about these we'll discuss over email.

Trunk is temporarily frozen now pending a customer delivery. When trunk unfreezes, we will put this into the merge queue.

Paul, 10290a does not compile with Java 8.

Constantin Asofiei wrote:

Paul, 10290a does not compile with Java 8.

I think it's because of String.isBlank() and String.repeat() methods. I might be wrong but I think that they are available starting with a newer version than 8. I will look for a replacement.

Paul Bodale wrote:

Constantin Asofiei wrote:

Paul, 10290a does not compile with Java 8.

I think it's because of String.isBlank() and String.repeat() methods. I might be wrong but I think that they are available starting with a newer version than 8. I will look for a replacement.

for repeat, see StringHelper.repeatChar(). For isBlank, trim().isEmpty() should work or add a helper to com.goldencode.util.StringHelper.

Constantin Asofiei wrote:

for repeat, see StringHelper.repeatChar(). For isBlank, trim().isEmpty() should work or add a helper to com.goldencode.util.StringHelper.

Committed rev. 16061 on branch 10290a that removes dependencies on those methods. I locally changed the java version to java 8 and then I ran ./gradlew all and the build succeeded.

Thanks for finding this out!

I am still investigating but I found that the problem comes from the fact that for this specific case, the options are dropped before they get a chance to be processed in literals.rules.

The options are dropped because of the rules found in constant_expressions.rules

Those rules appear to be an optimization of the exact case that Teo found out where 2 string literals are concatenated.

Created branch 10290b. I will modify this file so that we'll keep this optimization but take into account the string literal options.

Please merge 10920b after 9445a.