-
Notifications
You must be signed in to change notification settings - Fork 15
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
Feature/entry sections #126
Conversation
merge: merge of all commits until 2/1/2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good to me
OwO Imagine we have this code: define website:
port: 5030
trigger:
# </> Instead of getting the new SectionConfiguration()
.addOption("port", false, input -> {
// input is a String
/*
We could return an optional here:
- If the optional is empty, then an error occurred and SectionConfiguration will return false
- Else, it will attribute the parsed optional to this entry key and will return it when using getOption()
*/
try {
return Optional.of(Integer.parseInt(input));
} catch (Exception ex) {
logger.error("Invalid integer input: " + input);
return Optional.empty();
}
}); |
.addOption("port", false, input -> {
return Integer.parseInt(input);
}); This should be better. Optionals are important but only skript-parser should deal with it mainly. When getting options, we simply return a |
While the latest commit does not add the proposed feature directly, it makes it possible to do so. I will push the last commit soon, which will add the proposed feature and make it even easier to handle. Furthermore, I need to add Javadoc, a better explanation of what I'm doing and overall code improvements. The code is still somewhat all over the place but I'm way happier with what I created now than the old system. It's way more advanced than Skript's currently and allows developers to easily extend on the current system, which already provides many tools. I'm currently not really asking for a review, I will do that when I'm done with the last changes. |
@Mwexim any news about that PR ? |
This is almost ready to be merged honestly. Just a quick sweep/refactoring is necessary to make it more user-friendly, as well as some Javadocs explaining how everything works. |
src/main/java/io/github/syst3ms/skriptparser/lang/entries/SectionConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/syst3ms/skriptparser/lang/entries/SectionConfiguration.java
Show resolved
Hide resolved
I think returning an Optional for getValue could be nice, as the entries may not be present, rather than null. Also a cool feature would be allowing a pattern to be placed in the add entry methods, to allow for a pattern to be used to parse the key. like |
src/main/java/io/github/syst3ms/skriptparser/lang/entries/SectionConfiguration.java
Show resolved
Hide resolved
So I refactored a lot of code, resulting in breaking changes. I'm sorry to do this abruptly but I just couldn't stand the old implementation. If you are just implementing this, the changes will be minimal as only some methods have changed. I also added a lot of Javadoc and more descriptive error messages to further help future developers along. |
Entry sections are sections that require a specific set of entries to act as some sort of configuration.
Skript's command syntax is the perfect example.
We can immediately see the key features of the new
SectionConfiguration
.CodeSection
inside (like the trigger entry).The
SectionConfiguration
class essentially makes the creation of these configuration sections easy, handling all possibilities, error messages and more.This pull request is not completed yet as I still feel like some refinements should be made.