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.

Sign up for GitHub

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

Jump to bottom

🚀 Add Change Modes to expressions in docs #5687

Closed
wants to merge 3 commits into from
Closed

🚀 Add Change Modes to expressions in docs #5687

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ff0b987
Add Accepted ChangeModes to docs
AyhamAl-Ali May 10, 2023
b37394f
Add TODO comment
AyhamAl-Ali May 10, 2023
f86f829
Update src/main/java/ch/njol/skript/lang/Expression.java
AyhamAl-Ali Mar 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/main/java/ch/njol/skript/doc/AcceptedChangeModes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* This file is part of Skript.
*
* Skript is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Skript is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Skript. If not, see <http://www.gnu.org/licenses/>.
*
* Copyright Peter Güttinger, SkriptLang team and contributors
*/
package ch.njol.skript.doc;

import ch.njol.skript.classes.Changer;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* Provides a list of accepted {@link ch.njol.skript.classes.Changer.ChangeMode}s to be used in documentation for annotated
* elements.
*/
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Documented
public @interface AcceptedChangeModes {
Copy link
Member

Choose a reason for hiding this comment

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

what do you think about just taking in a string - might give developers more freedom for more complicated cases

for example, let's say we're annotating ExprName
@AcceptedChangeModes("set (only for display/tablist names)")

Copy link
Member Author

Choose a reason for hiding this comment

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

I would prefer this approach over current solution for freedom and ease of implementation however, this means we have to add this annotation to all current expressions manually which I am okay with but just to keep in mind.


public Changer.ChangeMode[] value();

}
58 changes: 41 additions & 17 deletions src/main/java/ch/njol/skript/doc/HTMLGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import ch.njol.skript.Skript;
import ch.njol.skript.SkriptAPIException;
import ch.njol.skript.classes.Changer;
import ch.njol.skript.classes.ClassInfo;
import ch.njol.skript.lang.Condition;
import ch.njol.skript.lang.Effect;
Expand All @@ -31,6 +32,7 @@
import ch.njol.skript.lang.function.Functions;
import ch.njol.skript.lang.function.JavaFunction;
import ch.njol.skript.lang.function.Parameter;
import ch.njol.skript.lang.util.SimpleExpression;
import ch.njol.skript.registrations.Classes;
import com.google.common.base.Joiner;
import com.google.common.collect.Lists;
Expand All @@ -46,6 +48,7 @@
import java.nio.charset.StandardCharsets;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.Date;
import java.util.Iterator;
Expand All @@ -64,6 +67,8 @@ public class HTMLGenerator {
private static final String SKRIPT_VERSION = Skript.getVersion().toString().replaceAll("-(dev|alpha|beta)\\d*", ""); // Filter branches
private static final Pattern NEW_TAG_PATTERN = Pattern.compile(SKRIPT_VERSION + "(?!\\.)"); // (?!\\.) to avoid matching 2.6 in 2.6.1 etc.
private static final Pattern RETURN_TYPE_LINK_PATTERN = Pattern.compile("( ?href=\"(classes\\.html|)#|)\\$\\{element\\.return-type-linkcheck}");
private static final Pattern DOCS_PASSED_FOLDERS_PATTERN = Pattern.compile("css|js|assets");
private static final Pattern DOCS_ACCEPTED_FILES_PATTERN = Pattern.compile("(?i)(.*)\\.(html?|js|css|json)");

private final File template;
private final File output;
Expand Down Expand Up @@ -210,9 +215,9 @@ public int compare(@Nullable JavaFunction<?> o1, @Nullable JavaFunction<?> o2) {
*/
@SuppressWarnings("unchecked")
public void generate() {
for (File f : template.listFiles()) {
if (f.getName().matches("css|js|assets")) { // Copy CSS/JS/Assets folders
String slashName = "/" + f.getName();
for (File file : template.listFiles()) {
if (DOCS_PASSED_FOLDERS_PATTERN.matcher(file.getName()).matches()) { // Copy CSS/JS/Assets folders
String slashName = "/" + file.getName();
File fileTo = new File(output + slashName);
fileTo.mkdirs();
for (File filesInside : new File(template + slashName).listFiles()) {
Expand All @@ -223,7 +228,7 @@ public void generate() {
writeFile(new File(fileTo + "/" + filesInside.getName()), readFile(filesInside));
}

else if (!filesInside.getName().matches("(?i)(.*)\\.(html?|js|css|json)")) {
else if (!DOCS_ACCEPTED_FILES_PATTERN.matcher(filesInside.getName()).matches()) {
try {
Files.copy(filesInside, new File(fileTo + "/" + filesInside.getName()));
} catch (IOException e) {
Expand All @@ -233,23 +238,23 @@ else if (!filesInside.getName().matches("(?i)(.*)\\.(html?|js|css|json)")) {
}
}
continue;
} else if (f.isDirectory()) // Ignore other directories
} else if (file.isDirectory()) // Ignore other directories
continue;
if (f.getName().endsWith("template.html") || f.getName().endsWith(".md"))
if (file.getName().endsWith("template.html") || file.getName().endsWith(".md"))
continue; // Ignore skeleton and README

Skript.info("Creating documentation for " + f.getName());
Skript.info("Creating documentation for " + file.getName());

String content = readFile(f);
String content = readFile(file);
String page;
if (f.getName().endsWith(".html"))
if (file.getName().endsWith(".html"))
page = skeleton.replace("${content}", content); // Content to inside skeleton
else // Not HTML, so don't even try to use template.html
page = content;

page = page.replace("${skript.version}", Skript.getVersion().toString()); // Skript version
page = page.replace("${skript.build.date}", new SimpleDateFormat("dd/MM/yyyy").format(new Date())); // Build date
page = page.replace("${pagename}", f.getName().replace(".html", ""));
page = page.replace("${pagename}", file.getName().replace(".html", ""));

List<String> replace = Lists.newArrayList();
int include = page.indexOf("${include"); // Single file includes
Expand Down Expand Up @@ -375,7 +380,7 @@ else if (!filesInside.getName().matches("(?i)(.*)\\.(html?|js|css|json)")) {
generate = page.indexOf("${generate", nextBracket);
}

String name = f.getName();
String name = file.getName();
if (name.endsWith(".html")) { // Fix some stuff specially for HTML
page = page.replace("\t", "&nbsp;&nbsp;&nbsp;&nbsp;"); // Tab to 4 non-collapsible spaces
assert page != null;
Expand Down Expand Up @@ -493,12 +498,12 @@ private String generateAnnotated(String descTemp, SyntaxElementInfo<?> info, @Nu
}
desc = desc.replace("${element.events}", Joiner.on(", ").join(eventLinks));
}
desc = desc.replace("${element.events-safe}", events == null ? "" : Joiner.on(", ").join((events != null ? events.value() : null)));
desc = desc.replace("${element.events-safe}", events == null ? "" : Joiner.on(", ").join(events.value()));

// RequiredPlugins
RequiredPlugins plugins = c.getAnnotation(RequiredPlugins.class);
desc = handleIf(desc, "${if required-plugins}", plugins != null);
desc = desc.replace("${element.required-plugins}", plugins == null ? "" : Joiner.on(", ").join((plugins != null ? plugins.value() : null)));
desc = desc.replace("${element.required-plugins}", plugins == null ? "" : Joiner.on(", ").join(plugins.value()));

// Return Type
ClassInfo<?> returnType = info instanceof ExpressionInfo ? Classes.getSuperClassInfo(((ExpressionInfo<?,?>) info).getReturnType()) : null;
Expand All @@ -511,6 +516,25 @@ private String generateAnnotated(String descTemp, SyntaxElementInfo<?> info, @Nu
// desc = desc.replace("${element.by-addon}", addon);
desc = handleIf(desc, "${if by-addon}", false);

// Accepted Changers
// TODO grab the accepted data type as well (find a way to include that in annotation as well)
if (info instanceof ExpressionInfo<?,?>) {
List<Changer.ChangeMode> acceptedChangeModes = new ArrayList<>(4); // 4 is common, SET/ADD/REMOVE/DELETE
AcceptedChangeModes annotation = c.getAnnotation(AcceptedChangeModes.class);
if (annotation != null) {
acceptedChangeModes = Arrays.stream(annotation.value()).toList();
} else {
try {
acceptedChangeModes = ((SimpleExpression<?>) info.getElementClass().newInstance()).getAcceptedChangeModes().keySet().stream().toList();
Copy link
Member

Choose a reason for hiding this comment

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

i am a little weary of this given that expressions really should be initialized before having anything done on them - this may very well lead to inaccurate results

} catch (Exception ignored) {}
}

desc = handleIf(desc, "${if accepted-change-modes}", acceptedChangeModes.size() > 0);
desc = desc.replace("${element.accepted-change-modes}", acceptedChangeModes.size() == 0 ? "" :Joiner.on(", ").join(acceptedChangeModes.toArray()));
} else {
desc = handleIf(desc, "${if accepted-change-modes}", false);
}

// New Elements
desc = handleIf(desc, "${if new-element}", NEW_TAG_PATTERN.matcher((since != null ? since.value() : "")).find());

Expand Down Expand Up @@ -625,12 +649,12 @@ private String generateEvent(String descTemp, SkriptEventInfo<?> info, @Nullable
}
desc = desc.replace("${element.events}", Joiner.on(", ").join(eventLinks));
}
desc = desc.replace("${element.events-safe}", events == null ? "" : Joiner.on(", ").join((events != null ? events.value() : null)));
desc = desc.replace("${element.events-safe}", events == null ? "" : Joiner.on(", ").join(events.value()));

// Required Plugins
String[] requiredPlugins = info.getRequiredPlugins();
desc = handleIf(desc, "${if required-plugins}", requiredPlugins != null);
desc = desc.replace("${element.required-plugins}", Joiner.on(", ").join(requiredPlugins == null ? new String[0] : requiredPlugins));
desc = desc.replace("${element.required-plugins}", requiredPlugins == null ? "" : Joiner.on(", ").join(requiredPlugins));

// New Elements
desc = handleIf(desc, "${if new-element}", NEW_TAG_PATTERN.matcher(since).find());
Expand Down Expand Up @@ -729,12 +753,12 @@ private String generateClass(String descTemp, ClassInfo<?> info, @Nullable Strin
}
desc = desc.replace("${element.events}", Joiner.on(", ").join(eventLinks));
}
desc = desc.replace("${element.events-safe}", events == null ? "" : Joiner.on(", ").join((events != null ? events.value() : null)));
desc = desc.replace("${element.events-safe}", events == null ? "" : Joiner.on(", ").join(events.value()));

// Required Plugins
String[] requiredPlugins = info.getRequiredPlugins();
desc = handleIf(desc, "${if required-plugins}", requiredPlugins != null);
desc = desc.replace("${element.required-plugins}", Joiner.on(", ").join(requiredPlugins == null ? new String[0] : requiredPlugins));
desc = desc.replace("${element.required-plugins}", requiredPlugins == null ? "" : Joiner.on(", ").join(requiredPlugins));

// New Elements
desc = handleIf(desc, "${if new-element}", NEW_TAG_PATTERN.matcher(since).find());
Expand Down
41 changes: 22 additions & 19 deletions src/main/java/ch/njol/skript/lang/Expression.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,36 +18,29 @@
*/
package ch.njol.skript.lang;

import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Spliterators;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

import org.bukkit.event.Event;
import org.bukkit.inventory.ItemStack;
import org.eclipse.jdt.annotation.Nullable;

import ch.njol.skript.Skript;
import ch.njol.skript.classes.Changer;
import ch.njol.skript.classes.Changer.ChangeMode;
import ch.njol.skript.classes.Changer.ChangerUtils;
import org.skriptlang.skript.lang.converter.Converter;
import ch.njol.skript.conditions.CondIsSet;
import ch.njol.skript.lang.util.ConvertedExpression;
import ch.njol.skript.lang.util.SimpleExpression;
import ch.njol.skript.log.BlockingLogHandler;
import ch.njol.skript.log.ErrorQuality;
import ch.njol.skript.log.LogHandler;
import ch.njol.skript.log.SkriptLogger;
import ch.njol.skript.registrations.Classes;
import ch.njol.skript.util.slot.Slot;
import ch.njol.util.Checker;
import org.bukkit.event.Event;
import org.bukkit.inventory.ItemStack;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
import org.skriptlang.skript.lang.converter.Converter;

import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Optional;
import java.util.Spliterators;
import java.util.stream.Stream;
Expand Down Expand Up @@ -291,15 +284,25 @@ default Optional<T> getOptionalSingle(Event e) {
public Class<?>[] acceptChange(ChangeMode mode);

/**
* Tests all accepted change modes, and if so what type it expects the <code>delta</code> to be.
* Tests all accepted change modes, and if so what data type it expects the <code>delta</code> to be.
* Keep in mind that this can't be very accurate if a change mode requires a specific condition to be met
* this will not be able to grab that most likely, therefore {@link ch.njol.skript.doc.AcceptedChangeModes} annotation
* exists to override the accepted change modes value in docs however, currently that annotation is missing the accepted data type.
*
* Skript errors that occur during testing {@link #acceptChange(ChangeMode)} method will be ignored.
* @return A Map contains ChangeMode as the key and accepted types of that mode as the value
*/
default Map<ChangeMode, Class<?>[]> getAcceptedChangeModes() {
HashMap<ChangeMode, Class<?>[]> map = new HashMap<>();
for (ChangeMode cm : ChangeMode.values()) {
Class<?>[] ac = acceptChange(cm);
if (ac != null)
map.put(cm, ac);
LogHandler logHandler = SkriptLogger.startLogHandler(new BlockingLogHandler());
Map<ChangeMode, Class<?>[]> map = new HashMap<>();
try {
for (ChangeMode cm : ChangeMode.values()) {
Class<?>[] ac = acceptChange(cm);
if (ac != null)
map.put(cm, ac);
}
} finally {
logHandler.stop();
AyhamAl-Ali marked this conversation as resolved.
Show resolved Hide resolved
}
return map;
}
Expand Down