-
-
Notifications
You must be signed in to change notification settings - Fork 414
Docs categories #8194
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
base: dev/feature
Are you sure you want to change the base?
Docs categories #8194
Conversation
# Conflicts: # src/main/java/ch/njol/skript/doc/JSONGenerator.java
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.
A syntax should be able to belong to multiple categories
I might also like to see a tree structure for categories:
|
Co-authored-by: SirSmurfy2 <[email protected]>
Co-authored-by: SirSmurfy2 <[email protected]>
Co-authored-by: SirSmurfy2 <[email protected]>
…re/grouped-docs # Conflicts: # src/main/java/ch/njol/skript/doc/JSONGenerator.java
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.
From a quick look 🙂
src/main/java/org/skriptlang/skript/bukkit/damagesource/elements/CondWasIndirect.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/damagesource/elements/ExprCausingEntity.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/damagesource/elements/ExprCreatedDamageSource.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/damagesource/elements/ExprCausingEntity.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/damagesource/elements/ExprSecDamageSource.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/registration/SyntaxOrigin.java
Outdated
Show resolved
Hide resolved
Co-authored-by: sovdee <[email protected]>
for (String keyword : impl.keywords()) { | ||
if (lower.contains(keyword)) { | ||
options.add(getCategoryJson(category)); | ||
break; | ||
} | ||
} | ||
} |
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.
Was randomly going through the wip updated docs page again and noticed "category" checked it out saw that timespan and beacon effect are classified as apart of the "Display" category.
I decided to go check into this, this is caused by the usage guessing assignments with contains
beacon effect contains display
and timespan contains displayed
.
Is this behavior planned to stay or do you plan to eventually move away from automatic assignments.
imo if something like this is to stay it should be checked against something more reliable
- Modules, which is already done with Categorizable
- Syntax patterns and not descriptions/names
%display%
vs.display

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.
I think excluding the description/examples, searching with \b
word boundaries, and specifically looking for types in the syntax patterns would be good improvements.
On the docs side, I'd like to see these as selectable categories (think shopping websites) rather than exclusive lists of syntaxes. One syntax may naturally belong to multiple categories, and having a way to select various categories for display I think would improve the UI.
Problem
Currently documentation is hard to index as all syntax elements are usually in a giant list with no possibility of organizing by type.
Solution
Allows syntax to be organized in
docs.json
by a few common categories. Here are some examples.These categories can be expanded on by (official) addons, in case a docs site needs them. Extending is as easy as implementing
Category
and adding your own fields usingCategory#of
.To allow getting the module with which a syntax was registered, a new method was added to
AddonModule
for mass registering syntax using the current module. This method is recommended instead of registering to a registry directly. The damage source module has been converted to the new registration API to showcase this.Testing Completed
Manual testing.
Supporting Information
Adds a new field to the
docs.json
spec calledcategory
.Completes: none
Related: none