Skip to content
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

Cobol lexer eol #284

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

NanoByte011
Copy link

Allows Cobol Lexer to respect Style 12 for EOL terminators (such as period in COBOL)

  • Code enhanced
  • Normalized whitespace code style for the file
  • Used AStyle to format LexCOBOL.cxx file

Michael Braams and others added 3 commits October 15, 2024 10:19
Updated Cobol Lexer to respect EOL Styles
Use consistent code styling and whitespace in file.
Update AllStyles Test to include EOL Style Test

Ran astyle on LexCOBOL.cxx to format file properly
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the whitespace changes on the first commit! Recommend hiding whitespace to see the actual code changes for this styling on this one commit. The next commits clean up whitespace issues so you can turn on view whitespace to review those commits

@nyamatongwe
Copy link
Member

nyamatongwe commented Oct 15, 2024

It's unclear just what this change is trying to do. The SCE_C_STRINGEOL style was defined by the cpp lexer to show single-line strings that don't have a terminating quote. See lexilla/test/examples/cpp/AllStyles.cxx for an example. The example added by this patch doesn't appear to be a string. If a new style is needed then the COBOL lexer should probably switch from using the cpp SCE_C_* lexical classes and define its own.

Reformatting the code makes it more difficult to understand the functional change and isn't needed.

@NanoByte011
Copy link
Author

Hmm I see. The intent here is to style EOL Terminators for the language.

C++ ends a statement with semi-colon ;
COBOL ends a statement with a period .

But it looks like that's not how SCE_C_STRINGEOL is used for C in the way we are using it for a terminating statement character in COBOL.

image

Open to recommendations. If a new Style set is needed, that might be too much work for me to do for the Current Cobol Lexer at this time. I'm not sure who the original author for it was, we've been using a modified version of the Lexer for years since ScintillaNET 2.0 lol

Was hoping to get this change in so it's a permanent part of the Lexer and our editor. I am just upgrading things to the latest versions (Scintilla.NET 5.6.1, etc.)

@nyamatongwe nyamatongwe added the cobol Caused by the COBOL lexer label Oct 17, 2024
@nyamatongwe
Copy link
Member

Most lexers treat terminators as operators so they are styled the same. There are benefits to enhancing the visibility of terminators but adding a new lexical class will make COBOL behave differently to other lexers in a way that applications, themes, and users won't be expecting.

nyamatongwe added a commit that referenced this pull request Oct 27, 2024
…COBOL.

Include C++ headers, use unnamed namespace instead of static, remove inline, use nullptr.
Related to #284.
nyamatongwe added a commit that referenced this pull request Oct 27, 2024
…OBOL lexers reuse of

the SCE_C_* constants defined for the cpp lexer.
The values are the same to provide binary compatibility.
Related to #284.
nyamatongwe added a commit that referenced this pull request Oct 27, 2024
…s, just the 3rd set of keywords

or known identifiers.
The value remains the same, 8.
Related to #284.
@nyamatongwe
Copy link
Member

Changed the COBOL lexer to use its own named style constants SCE_COBOL_* and renamed UUID with WORD3 as there are no UUIDs in COBOL and the lexer used that style for just another set of identifiers. Also performed some minor clean-ups including using an unnamed namespace instead of static as its safer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cobol Caused by the COBOL lexer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants