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

Allow setting properties inside test example files #62

Closed
nyamatongwe opened this issue Mar 5, 2022 · 14 comments
Closed

Allow setting properties inside test example files #62

nyamatongwe opened this issue Mar 5, 2022 · 14 comments
Labels
enhancement New feature or request testing Improvement to testing

Comments

@nyamatongwe
Copy link
Member

Requiring a new subdirectory for each property change test is unwieldy. It would be better to allow test example files to set properties that differ from SciTE.properties. These could be placed in comments at the start of the test example file similar to:

# Test with f-strings disabled: [|lexer.python.strings.f=0|]

The [|...|] syntax was chosen as these sequences are unusual so should avoid unexpected matches.

An implementation is available from 62InlineProperties.patch.

Adding a subdirectory may still be worthwhile for properties that have complex effects or interactions.

@nyamatongwe nyamatongwe added enhancement New feature or request testing Improvement to testing labels Mar 5, 2022
@nyamatongwe nyamatongwe changed the title Allow setting properties inside text example files Allow setting properties inside test example files Mar 5, 2022
@nyamatongwe
Copy link
Member Author

On #60, @mpheath mentioned that Markdown doesn't have comments. Other languages also lack comments including data languages like JSON. The above syntax is likely safe unless there are languages in which [...] or | always have special meanings even in quoted strings.

@mpheath
Copy link
Contributor

mpheath commented Mar 7, 2022

Makes me consider the SciTE.properties file and how it is basically a skeleton of settings for testing with TestLexer. This can make viewing in SciTE as an unclear indication of how the test file looks in a test user state. So perhaps a Theme.properties or another fixed name like that can be imported by the SciTE.properties for the good of viewing in SciTE and that TestLexer could ignore the import line and ignore the Theme.properties file so that it only uses the skeleton of settings.

This idea would allow Theme.properties to have stylings, with background colourings..., suitable to see the issues or successes in SciTE. Even if not having Theme.properties commited, it may allow a tester to make the file, add the import line and test with it without TestLexer complaining about Theme.properties, as it does with extra unknown files present.

Theme.properties could also possibly be any name defined by the import statement and that TestLexer will recognize, so can allow for multiple property files if needed. This may help for a imported file with the properties that are embedded in the source file to be read and viewed in SciTE. Yeah, starting to get into possible file modification to change the later properties etc, though the imported files are for SciTE and will not affect the operation of TestLexer. Testing of properties files in the folder properties that does not yet exist, could cause problems so perhaps atleast the imported files could require a prefix of SciTE so that they are separated from the test files and that no test file can have the SciTE prefix. I consider the prefix concept as more robust and could make it easier for TestLexer as it can ignore the SciTE prefixed files except for SciTE.properties.

This idea has not been tried so it is untested. Maybe too early to know if needed. Maybe too much, though can be regarded as optional to make use of.

P.S. This line for example:

style.python.11.3=fore:#EEAA80,italics,bold

is not needed for TestLexer in my opinon, so perhaps could be in Theme.properties if preferred. Perhaps with a more extensive theme suitable for testing purposes. Example with background colourings to show where styles start and end.

@mpheath
Copy link
Contributor

mpheath commented Mar 7, 2022

On #60, @mpheath mentioned that Markdown doesn't have comments...

I forgot about HTML tag usage for comments in Markdown, although the lexer does not process them as actual comments, so could be

<!-- `Test property: [|lexer.markdown.header.eolfill=1|]` -->

which should make the styling as

{0}<!-- {19}`Test property: [|lexer.markdown.header.eolfill=0|]`{0} -->{1}

For Markdown, it probably means little difference between HTML tags or just backquotes as it is going to be styled anyway. JSON in comparison might be tricky as styling can go red if is invalid JSON, so perhaps embed the properties as string values may possibly work, though probably undesirable to embed into the data itself.

@rdipardo
Copy link
Contributor

rdipardo commented Mar 7, 2022

Other languages also lack comments including data languages like JSON

[ . . . ] perhaps embed the properties as string values may possibly work

While there's no standard implementation of JSON comments beyond the json5 npm library, it seems LexJSON already offers this feature:

lexilla/lexers/LexJSON.cxx

Lines 125 to 126 in 5bb83ae

DefineProperty("lexer.json.allow.comments", &OptionsJSON::allowComments,
"Set to 1 to enable highlighting of line/block comments in JSON");

json_comments

@nyamatongwe
Copy link
Member Author

@mpheath :

So perhaps a Theme.properties or another fixed name like that can be imported by the SciTE.properties for the good of viewing in SciTE and that TestLexer could ignore

The simple properties file reader in TestLexers doesn't process import statements so magically anticipated your desire.

For testing the .properties lexer, the extension filter in TestDirectory will need updating to test files unless they match some pattern like SciTE*.properties.

@nyamatongwe
Copy link
Member Author

Another approach would add an optional companion .properties file to each test, so testing HeaderEOLFill_0.md would add properties from HeaderEOLFill_0.md.properties. That avoids any syntactic issues at the cost of making the setting less visible when looking at the test example file.

For testing the properties lexer, a different extension can be used for the example files. .session is already used by SciTE for storing session data in .properties format.

@mpheath
Copy link
Contributor

mpheath commented Mar 8, 2022

@nyamatongwe

If import feature is wanted, then a new recognizable option would be TestLexer.properties and SciTE.properties. It should be obvious which file is for which program. The default line in SciTE.properties would be import TestLexer and TestLexer.properties would have no import lines to worry about even though as you state, import does not currently get processed.

SciTE does not know about HeaderEOLFill_0.md.properties unless you are referring to importing it, unless otherwise referring to properties read by TestLexer which may double the amount of files. Makes me now consider that the multiple import idea I mentioned previously might be a bad idea as it could lead to too many properties files. Or, perhaps you having second thoughts about the source embedded properties to be moved into multiple properties files.

New extension without any association does not seem like a benefit. I am uncertain on this new extension idea. My thoughts are to have it work with SciTE with existing loading techniques.

Whatever is implemented, if implemented, it should not be complex, else testing could become uncertain. That, I hope, can be mutually agreed.

@nyamatongwe
Copy link
Member Author

File name conditional expressions could be added inside SciTE.properties files. This avoids proliferating properties files and uses a single syntax for settings.

if $(= $(FileNameExt);HeaderEOLFill_1.md)
    lexer.markdown.header.eolfill=1

This isn't quite valid in SciTE but would be if $(FileNameExt) was set inside SciTEBase::ReadLocalPropFile before reading the local properties file.

propsLocal.Clear();
propsLocal.Set("FileNameExt", FileNameExt().AsUTF8().c_str());
propsLocal.Read(propfile, propfile.Directory(), filter, nullptr, 0);

If this was just for TestLexers, not for SciTE, a simpler syntax could be used similar to EditorConfig

[HeaderEOLFill_1.md]
lexer.markdown.header.eolfill=1

@mpheath
Copy link
Contributor

mpheath commented Mar 9, 2022

So presuming, SciTE.properties might be:

code.page=65001
lexer.*.md=markdown
fold=1

[HeaderEOLFill_1.md]
if $(= $(FileNameExt);HeaderEOLFill_1.md)
	lexer.markdown.header.eolfill=1

	# SCE_MARKDOWN_HEADER1: "#" - Level-one header
	style.markdown.6=fore:#5183C4,bold,$(font.monospace),eolfilled,back:#FFFFCC
	# SCE_MARKDOWN_HEADER2: "##" - Level-two header
	style.markdown.7=$(style.markdown.6),back:#CCFFFF

The [HeaderEOLFill_1.md] line is not needed though some kind of comment might be nice.

and TestLexer.properties or other name might be similar:

code.page=65001
lexer.*.md=markdown
fold=1

[HeaderEOLFill_1.md]
lexer.markdown.header.eolfill=1

Can be differenced with WinMerge or similar program to compare and update the common settings if needed. It will work with SciTE and TestLexer. Some duplication of settings with the 2 files, though should give good testing and viewing results.

Click to view image of SciTE with *HeaderEOLFill_0.md* viewed on the left and *HeaderEOLFill_1.md* viewed on the right:

TestLexer

Currently, I comment the if line in SciTE.properties to get the right side view, though if SciTEBase::ReadLocalPropFile is updated, should be automatic.

I like this idea as it should work well with the various options associated with the test files. 👍

Edit: Those 2 md files are also duplicates, just different naming, perhaps an idea to use the same file? I don't consider it would work good with the view in SciTE. Perhaps 2 separate md files is needed.

@mpheath
Copy link
Contributor

mpheath commented Mar 11, 2022

To save duplication of properties, could go back to the 1 file SciTE.properties to read.

Example:

code.page=65001
lexer.*.md=markdown
fold=1

if Test_1
	testfile=HeaderEOLFill_0.md
	lexer.markdown.header.eolfill=0

if Test_2
	testfile=HeaderEOLFill_1.md
	lexer.markdown.header.eolfill=1

if Test_Done
	done=true

if $(= $(FileNameExt);HeaderEOLFill_1.md)
	lexer.markdown.header.eolfill=1
	
	# Level-one header=6
	style.markdown.6=fore:#5183C4,bold,$(font.monospace),eolfilled,back:#FFFFCC
	# Level-two header=7
	style.markdown.7=$(style.markdown.6),back:#CCFFFF

TestLexer will check for lines starting with if Test_ and consider the following properties as a test option to occur with the testfile value, which is a filename. If the line if Test_Done is read by TestLexer, then it ends reading properties. The done=true is just a dummy property setting which does nothing.

SciTE should read all of the file and will exclude the sections after the lines starting with if Test_. I doubt if Test_... will be true in SciTE unless something I do not know about. This will still rely on the SciTEBase::ReadLocalPropFile update to process the if $(=... conditions.

Perhaps if Test_ could be if TestOption_ as it set options for that test file. The example is just a basic idea and could be improved.

@nyamatongwe
Copy link
Member Author

There could be an alias command alias HeaderEOLFill_1.md=HeaderEOLFill_0.md that introduces an alias for an existing file. TestLexers would accumulate a list of aliases and run the test over those aliases after running it over all the real files. When reading in a file, if its not present, then its alias (if any) is read instead. Conditionals would work the same as for real files.

code.page=65001
lexer.*.md=markdown
fold=1

alias HeaderEOLFill_1.md=HeaderEOLFill_0.md

if $(= $(FileNameExt);HeaderEOLFill_0.md)
	lexer.markdown.header.eolfill=0

if $(= $(FileNameExt);HeaderEOLFill_1.md)
	lexer.markdown.header.eolfill=1
	# Level-one header=6
	style.markdown.6=fore:#5183C4,bold,$(font.monospace),eolfilled,back:#FFFFCC
	# Level-two header=7
	style.markdown.7=$(style.markdown.6),back:#CCFFFF

When working on a particular file, a copy could be made with the alias name so it is easy to compare the two cases. Then when the work is complete, the copy is removed and not committed to git.

It might even be possible to add alias support to SciTE with a menu to choose between different aliases of a file and thus different features but that may be excessive.

@mpheath
Copy link
Contributor

mpheath commented Mar 12, 2022

I am settled on this at this point, unless anyone has some more ideas. Looks like the later ideas are worth testing.

The alias idea seems interesting and would like to see how that works. The alias set in Gui idea is too early to consider implementing, though is another interesting idea.

The if TEST_DONE case as default case could be done for testing. The other if TEST_ could have a trailing filename perhaps, example if TEST_HeaderEOLFill_0.md. I guess the alias could work with that filename too, similar to the if $(= $(FileNameExt);HeaderEOLFill_0.md) line.

Hopefully the warnings output can still be concise working with aliases. I am sure you can handle this. ;)

@nyamatongwe
Copy link
Member Author

File name testing and alias are separate features and alias can be implemented later as its not essential to achieve the goals here.

An implementation of if $(= $(FileNameExt);... and match is fromTestFileNameExt.patch.

It is sufficient for both selection statements in:

if $(= $(FileNameExt);AllStyles.md)
    lexer.markdown.header.eolfill=1

match Bug1216.md
    lexer.markdown.header.eolfill=1

I'll propose these on the SciTE mailing list.

nyamatongwe added a commit that referenced this issue Mar 15, 2022
…and "FileNameExt"

property in TestLexers to allow varying lexer properties over different files.
@nyamatongwe
Copy link
Member Author

Committed the basic 'match' feature with 2c9e004. This replaces the original plan of allowing properties to be set inside the example files.

'alias' may become a separate issue in the future if it seems beneficial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request testing Improvement to testing
Projects
None yet
Development

No branches or pull requests

3 participants