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

Changed include to support wildcards #346

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Changed include to support wildcards #346

wants to merge 1 commit into from

Conversation

davidsonbr
Copy link

I felt like the changes to ParseDirectives should be bundled with this commit, feel free to tell me otherwise.

@awilliamson
Copy link
Member

ParseDirectives for the last param should always be a table. Even if it just has a single item. I think having string/table isn't nice behaviour.


local preprocs_type = type( preprocs )
if preprocs_type == "string" then --1 specified preproc
local func = directives[ preprocs ] or SF.Preprocessor.directives[ preprocs ]
Copy link
Member

Choose a reason for hiding this comment

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

Lots of code blocks like this switching on type. As suggested, keep it as nil or a table. Try and optimise this to a single block, make use of ternary for nil checks etc.

@davidsonbr
Copy link
Author

ParseDirectives has been made into a single block.

@awilliamson
Copy link
Member

@EpicG Would this break anything you're working on? I remember us chatting about remaking the ParseDirectives to refactor it properly for chaining directive checking.

@Jazzelhawk
Copy link
Member

Was testing this a bit and came up with a few things:

You can't have more than two of the same statement, preprocessor.lua:125
err returned from ParseDirectives is a table, can't be concatenated in editor.lua:877
directive_include doesn't work properly when starting with *
Maybe change include directive example to show wildcards

@davidsonbr
Copy link
Author

Those are not the same statement, one is singular one is plural
Fixed err
fixed starting with *
Added example to show wildcards

@Jazzelhawk
Copy link
Member

I meant when you have two of the same directive, they are going to be overwritten in the directives table.
directives[ directive ] = args
If directive is equal to "include" then what ever was there before is just going to be overwritten.

@davidsonbr
Copy link
Author

Ohhhh I see now. gotcha. Will fix that asap.

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.

4 participants