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

Adds language files for QB64 (vanilla and $NOPREFIX mode) #133

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

Conversation

FellippeHeitor
Copy link

No description provided.

@FellippeHeitor
Copy link
Author

Oh dang... whitespace at EOL. Hold on.

@BenBE
Copy link
Contributor

BenBE commented Apr 19, 2020

Looking through the language file it looks good so far. There are some issues commonly popping up, that are usually easy to fix.

Let's dig in:

1) LangCheckTest::test_langfile with data set "qb64" ('/home/travis/build/GeSHi/gesh...64.php')
The following issues were found in /home/travis/build/GeSHi/geshi-1.0/tests/../src/geshi/qb64.php:
[WARNING] Language file contains an empty keyword list in $language_data['KEYWORDS'] for group 1!
[NOTICE]  Language file contains an keyword ('?') at $language_data['KEYWORDS'][3][23] which seems to be better suited for the symbols section!

The first warning is a note about the first keyword group, which is currently empty. Traditionally this keyword group contains control flow statements and other structural keywords (like SUB, FUNCTION, RETURN). Having that group empty is a waste of resources, as this takes up extra cycles without doing any highlighting work.

The notice below is a hint regarding one of the keywords listed in the second keyword group. GeSHi differentiates between keywords (mostly letters) and operators/symbols (usually no letters) as keywords are matched with additional boundary conditions. The entry ? should thus be moved into its own symbols group (if it really should stay separate).

Which brings up another point I noticed with that first language file: Some keywords begin with $ which is AFAIR totally fine in QBasic and also doesn't cause trouble for GeSHi per se, but you might consider to split those into separate keyword group. Doing so allows to optimize things a bit more (like having a optimized keyword locator regexp set in PARSER_CONTROL -> KEYWORDS -> DISALLOWED_BEFORE/AFTER).

The second language file basically complains about the same first two issues. The other issues reported are keywords that appear more than once in the language file and thus may cause ambiguity when highlighting. Also at times this can cause problems with the URL linking feature if keywords in one (later) group match on URLs generated for keywords in an earlier group.

I hope this helps so far.

@FellippeHeitor
Copy link
Author

Will work on those.

@FellippeHeitor
Copy link
Author

Oh, finally 👍

CHANGELOG Outdated
@@ -17,6 +17,7 @@ Version 1.0.9.1
* OpenSSH config (Kevin Ernst)
* roff (Artur Iwicki)
* Wolfram (Mysterious Light)
* QB64 (changes to Nigel McNie's work on qbasic.php made by Fellippe Heitor/QB64 Team)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just your name here is fine; list is alphabetical.

Should also start a new block for Version 1.0.9.2 (1.0.9.1 has been released already).

Copy link
Author

Choose a reason for hiding this comment

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

Cool, will change it.

Comment on lines 7 to 8
* Author: Nigel McNie (nigel@geshi.org)
* Copyright: (c) 2004 Nigel McNie (http://qbnz.com/highlighter/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just add your name here in that list. Usually there's no extra mod by line - everybody who considers himself to have contributed enough to feel the need to be mentioned may add himself as (co)author)..

Copy link
Author

Choose a reason for hiding this comment

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

Ok.

* Original file: qbasic.php
* Author: Nigel McNie (nigel@geshi.org)
* Copyright: (c) 2004 Nigel McNie (http://qbnz.com/highlighter/)
* Release Version: 1.0.9.0
Copy link
Contributor

Choose a reason for hiding this comment

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

1.0.9.2 (although this will be updated automagically when I prepare updates).

Copy link
Author

Choose a reason for hiding this comment

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

Aight.

* Release Version: 1.0.9.0
* Date Started: 2004/06/20
*
* QBasic/QuickBASIC language file for GeSHi.
Copy link
Contributor

Choose a reason for hiding this comment

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

QB64 langauge file … I presume?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah :-)

* -------------------------
* * Make sure all possible combinations of keywords with
* a space in them (EXIT FOR, END SELECT) are added
* to the first keyword group
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that there's a special PARSER_CONTROL (cf. vbscript language or bash:

'SPACE_AS_WHITESPACE' => false
) to tell GeSHi to treat any number of whitespace (tab+blanks) as one space. Thus EXIT FOR will still match if there was more than one tab or space to separate those keywords.

Copy link
Author

Choose a reason for hiding this comment

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

I'll check what's been done to the vbscript parser, thanks.

'COMMENT_MULTI' => array(),
'COMMENT_REGEXP' => array(
//Single-Line Comments using REM command
2 => "/\bREM.*?$/i",
Copy link
Contributor

Choose a reason for hiding this comment

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

Note sure here, but does REMOTE at the begin of a line really start a comment too? Or should it be /\bREM\b.*?$/i?

Copy link
Author

Choose a reason for hiding this comment

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

Regex is beyond me, but REM does require at least one space before comment starts. I trust that's what your proposed fix does, so I'll be adding it and testing it to see if it goes as intended. Thanks!

GESHI_NUMBER_FLT_NONSCI_F | GESHI_NUMBER_FLT_SCI_SHORT |
GESHI_NUMBER_FLT_SCI_ZERO,
'KEYWORDS' => array(
3 => array(
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a hard requirement, but keyword groups should be indexed starting at 1. Many generic stylesheets somewhat rely on that to provide reasonable highlighting even for new languages.

NB: Commonly grouping is roughly based on: kw1-> reserved words/control flow, kw2->built-in functions, kw3->common other stuff usually used (like system-provided variables), kw4…->all the other stuff.

Copy link
Author

@FellippeHeitor FellippeHeitor Apr 19, 2020

Choose a reason for hiding this comment

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

Yeah, because of the typical grouping convention I felt I should skip block 1 since for QBasic-based languages it's all keywords really, regardless of being control flow keywords (Nigel skipped block 1 for the original qbasic.php, leaving it blank - which caused issues with the CI and brought considerable confusion to me) but I'll reassign the proper sequential number to those starting at 1.

Comment on lines 3 to 12
* qb64np.php
* Mod by: Fellippe Heitor (fellippe@qb64.org)
* ----------
* Original file: qbasic.php
* Author: Nigel McNie (nigel@geshi.org)
* Copyright: (c) 2004 Nigel McNie (http://qbnz.com/highlighter/)
* Release Version: 1.0.9.0
* Date Started: 2004/06/20
*
* QBasic/QuickBASIC language file for GeSHi.
Copy link
Contributor

Choose a reason for hiding this comment

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

Cf. notes on the other file.

BTW: What's the diff between qb64 and qb64np?

Copy link
Author

Choose a reason for hiding this comment

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

With v1.4 QB64 introduced a metacommand ($NOPREFIX) that allows both _TITLE and TITLE to be used, reducing typing. File qb64np.php includes both prefixed and non-prefixed keywords, so users can highlight their code properly in the forum:

image

Is it possible to use $NOPREFIX as a trigger to allow a certain keyword group to be toggled? I thought that might have been too much to ask, hence the pair of language files.

Copy link
Contributor

Choose a reason for hiding this comment

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

The GeSHi 1.0 branch does not allow for much of contextual highlighting (parser limitation), while the GeSHi 1.1 branch (other repo) even allows for nesting languages and other crazy things. So splitting those two use-cases is fine.

If all keywords of a group should be matched with and without a certain prefix you might even play with the PARSER_CONTROL -> KEYWORDS -> BEFORE setting matching the usual boundary followed by an optional _?. Keywords themselves would be without the underscore in the keyword list only. But that's probably best for some separate PR.

Comment on lines 336 to 338
3 => 'http://www.qb64.org/wiki/index.php/{FNAMEU}',
4 => 'http://www.qb64.org/wiki/index.php/{FNAMEU}',
5 => 'http://www.qb64.org/wiki/index.php/{FNAMEU}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Please link HTTPS if you provide proper TLS.

Copy link
Author

Choose a reason for hiding this comment

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

Will do.

Comment on lines 257 to 258
3 => 'http://www.qb64.org/wiki/index.php/{FNAMEU}',
4 => 'http://www.qb64.org/wiki/index.php/{FNAMEU}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Please link to the HTTPS version as you provide TLS on your site.

Copy link
Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Contributor

@BenBE BenBE left a comment

Choose a reason for hiding this comment

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

LGTM. Sorry for the late feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants