Update Logtalk language syntax support
ClosedPublic

Authored by pmoura on Aug 4 2018, 10:57 AM.

Details

Summary

Update Logtalk language syntax support for the latest language specification (3.19.0).

Test Plan

Tested using the sample Logtalk source file available at:

https://github.com/LogtalkDotOrg/logtalk3/tree/master/coding/tests

Diff Detail

Repository
R216 Syntax Highlighting
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
pmoura created this revision.Aug 4 2018, 10:57 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptAug 4 2018, 10:57 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
pmoura requested review of this revision.Aug 4 2018, 10:57 AM
cullmann requested changes to this revision.Aug 12 2018, 12:45 PM

Thanks for the contribution.
I think the version=... should just be an integer, just increment it by one.
Beside that, it would be very nice to have some test file in the autotests folder like for other languages for this with the new features you cover (at least some parts would already be nice).

This revision now requires changes to proceed.Aug 12 2018, 12:45 PM
pmoura updated this revision to Diff 39556.Aug 13 2018, 8:22 AM

Update Logtalk language syntax support per reviewer request.

Could you provide a test file for the autotests unit tests?

Would adding the sample Logtalk source file I mentioned renamed to highlight.lgt to the autotests/input folder do?

Yes, and then let it run and review the results + add the reference files. That would be more than enough and we ensure with that it doesn't regress on later refactoring/bugfixes.
Thanks!

pmoura updated this revision to Diff 39558.Aug 13 2018, 8:33 AM

Added autotests/input/highlight.lgt file.

How do generate the reference files? No idea of what you mean by " let it run".

Just curious: The test file looks good to me, but it looks to me that it also contains some redundancy: Think of it as follows: We use this for unit testing. As such, the best case is that a test file tests all the corner cases, but ideally only once. If a test fails, then we get a diff with the changed highlighting information. And if the file is 400 lines of code, checking for regressions is much more work compared to if the file only contains e.g. 50 lines. So: would it be possible to strip it down a bit? And also: Is this file freely usable, e.g. licensed under MIT? Would be nice to add a license comment in the first line.

The test file tests everything that should be highlighted properly. There is no redundancy in it. It is large due to testing all built-in language features, including all that would be called keywords in other languages. It is the same test file that is used to check Logtalk syntax highlighting for two dozens of text editors and syntax highlighters. The original file is part of the Logtalk distribution and thus uses the Apache License 2.0. This version, highlight.lgt can be re-licensed using the license preferred for your project. Note that the contents of the file are just for testing highlighting and make no sense as an actual compilable source code.

If there is no redundancy, I am ok with that file. The license should be stated in it explicitly, will make it later easier to understand why we can use that.
For the reference generation: after a run, you will get told that stuff diffs, e.g. you do

make test

and the tests will fail

You then can look at the output in the autotests folder and copy the matching output files to the references folder in the framework.

Ok, perfect, thanks! I think "Apache License 2.0" is fine as well. It's just important that we do know the license, because otherwise it's a mess over time.

How to create the reference. When compiling KSyntaxHighlighting, you then write:

  • make
  • make test

make test will run all unit tests, and create new files in the build directory:

  • highlight.lgt.fold
  • highlight.lgt.html
  • highlight.lgt.ref

These can be copied into the source folder by running in the build folder: ./autotests/update-reference-data.sh

pmoura updated this revision to Diff 39566.Aug 13 2018, 9:06 AM

Added license to Logtalk highlighting test file.

dhaumann accepted this revision.Aug 13 2018, 9:06 AM

Shall we integrate this, or can you commit yourself?

I'm on macOS and I only forked from GitHub the syntax-highlighting repo. I don't have the necessary infra-structure to compile KSyntaxHighlighting and generate the reference files. I would very much appreciate if you can take it from here. I did test the updated syntax highlight support for Logtalk by replacing the data/syntax/logtalk.xml in a borrowed Ubuntu box, opening the test file, and checking that everything was highlight perfectly.

cullmann accepted this revision.Aug 13 2018, 9:12 AM
This revision is now accepted and ready to land.Aug 13 2018, 9:12 AM
This revision was automatically updated to reflect the committed changes.

Thanks for your help!

Thanks goes to do - and sorry for the delay on our side.