✨ feat: add a new `bs.string` module #283

aure31 · 2024-11-30T12:45:12Z

Tasks to do before merging

I accept to publish my contribution under MPL v2 License
My pull request is linked to an existing issue
I have added my contribution to the changelog
If my pull request is a new or modify an existing feature:
- I have documented my contribution (/docs)
- I have added or updated the header of the features' root function I contribute
- I have tested my contribution

and implementation of the `index_of` and `lenth` feature

aure31 · 2024-11-30T19:42:29Z

close #191

…ries (mcbookshelf#291)

…mcbookshelf#294)

and implementation of the `index_of` and `lenth` feature

+ starting the lower and upper case

and come fix

aksiome

Hey really quick review before this gets too big. I have'nt reviewed everything but it's a start

core/templates/load/string/concat.jinja

core/templates/load/string/concat_macros.jinja

meta/versions.json

modules/bs.string/README.md

modules/bs.string/data/bs.string/function/__load__.mcfunction

modules/bs.string/data/bs.string/function/concat/dispatch.mcfunction

modules/bs.string/data/bs.string/function/lower_case/fast/loop.mcfunction

modules/bs.string/data/bs.string/function/upper_case/fast/loop.mcfunction

…ction

…of the inverse table

aksiome

Ok i'm really nitpicking here, i mainly reviewed the style and not the logic. For the logic I'll need more time to run the tests and all. Also there are some remaining say and tellraw, i'm sure it's because it's not completly finished and ready for the last review, but i commented some of them (not all). A small cleanup cannot hurt i guess 👍

docs/modules.md

aksiome · 2025-01-26T16:32:57Z

docs/modules/string.md

@@ -0,0 +1,100 @@
+# 🔠 String
+
+**`#bs.string:help`**


We try to have a small description, a banner image, and a quote for our module. Nothing is mandatory except the description, but it's better with all of them

how you find the image for the module with a black and white variant

Either have a png with transparent background, or make 2 versions. If you don't really feel like doing some image design we'll try to do it (either Theo or me)

docs/modules/string.md

modules/bs.string/data/bs.string/function/find/find.mcfunction

modules/bs.string/data/bs.string/function/find/inverse/inverse.mcfunction

modules/bs.string/data/bs.string/function/find/inverse/up.mcfunction

modules/bs.string/data/bs.string/function/find/precompute.mcfunction

modules/bs.string/templates/concat.jinja

aksiome

Globally i think we should avoid mixing storage and macro. This is because it's really hard to tell if some parameters are to be used dynamically or not. For example occurence may be used with a constant value but some people might want to use it dynamically and it adds extra complexity. I think we should only use storage for the whole module. Also some tests are failing, and i think some function could be changed a bit. Here are my propositions:

find should be renamed find_all (still returns a list) and we should have a simple find that returns only the first occurence as an int from the function. This mean that the command can easily be used with if function.
insert can be reworked easily into something like replace_range / substr_replace which does the same and more with a similar implementation (see the details after)

aksiome · 2025-03-02T16:59:59Z

docs/modules/string.md

+# 🔠 String
+
+**`#bs.string:help`**
+


In the documentation a lot of sentences are missing their . at the end.

Verbs should be imperative. For example in uppercase/lowercase, Converts should be Convert instead.

In multiple examples the description does not have an uppercase at the start of the sentence.

Functions should be sorted in alphabetical order.

Storage inputs/outputs should use treeview (see other modules like block) if more than 1, and examples should also work this way. For example in replace:
# avoid data modify storage bs:in string.replace.str set value "hello world" data modify storage bs:in string.replace.old set value "world" data modify storage bs:in string.replace.new set value "minecraft" # use instead to encourage people to use only the base path (string.replace) to override the whole compound and avoid unintended side effects when multiple pack use bookshelf data modify storage bs:in string.replace set value {str: "hello world", old: "world", new: "minecraaft"}

aksiome · 2025-03-02T17:09:00Z

docs/modules/string.md

+  **Strorage `bs:out string.replace`**: {nbt}`string` The replaced string
+```
+
+_replace world by minecraft:_


world and minecraft missing `` => world and minecraft

aksiome · 2025-03-02T17:09:50Z

docs/modules/string.md

+data modify storage bs:in string.replace.old set value "world"
+data modify storage bs:in string.replace.new set value "minecraft"
+
+function #bs.string:replace {type:'fast'}


The type macro should not be here

aksiome · 2025-03-02T17:14:39Z

modules/bs.string/data/bs.string/function/__unload__.mcfunction

+scoreboard objectives remove bs.out
+scoreboard objectives remove bs.ctx
+
+#data remove storage bs:in string


You should remove both input and outputs when unloading the module also the const score

aksiome · 2025-03-02T17:15:36Z

modules/bs.string/data/bs.string/function/__load__.mcfunction

+scoreboard objectives add bs.ctx dummy [{"text":"BS ","color":"dark_gray"},{"text":"Ctx","color":"aqua"}]
+scoreboard objectives add bs.const dummy [{"text":"BS ","color":"dark_gray"},{"text":"Const","color":"aqua"}]
+
+#set constant


Not really consistent with other comments, we should always add a space after #. Also not sure this one is really needed since the code is self explanatory. We don't really have conventions for comments but i think it's best to be consistent across a module (like for example if you use uppercase at the start of comments use it everywhere)

aksiome · 2025-03-02T19:04:59Z

modules/bs.string/data/bs.string/function/find/count/up.mcfunction

+# For more details, refer to the MPL v2.0.
+# ------------------------------------------------------------------------------------------------------------
+
+data modify storage bs:out string.find append from storage bs:ctx x


As said after in find avoid using x and use the score instead

aksiome · 2025-03-02T19:06:13Z

modules/bs.string/data/bs.string/function/find/count/count.mcfunction

+data modify storage bs:ctx _.ltr set string storage bs:ctx _.test -1
+execute store success score #t bs.ctx run data modify storage bs:ctx _.test set from storage bs:in string.find.needle
+
+data modify storage bs:ctx z set from storage bs:ctx y


not needed directly use y directly in match_pattern instead

aksiome · 2025-03-02T19:06:47Z

modules/bs.string/data/bs.string/function/find/count/skip.mcfunction

+#
+# For more details, refer to the MPL v2.0.
+# ------------------------------------------------------------------------------------------------------------
+


Same as for normal/skip

aksiome · 2025-03-02T19:09:20Z

modules/bs.string/data/bs.string/function/split/split.mcfunction

+# - Any modifications must be documented and disclosed under the same license
+#
+# For more details, refer to the MPL v2.0.
+# ------------------------------------------------------------------------------------------------------------


missing new line. Also im lazy and i think a lot of code is shared with find, so i'll just say everything for find applies here

aksiome · 2025-03-02T19:11:47Z

modules/bs.string/data/bs.string/function/replace/replace.mcfunction

+# For more details, refer to the MPL v2.0.
+# ------------------------------------------------------------------------------------------------------------
+
+#reset


comment style. Also im lazy so i'll just say that everything from find applies here

creation of bs.string

bd06af0

and implementation of the `index_of` and `lenth` feature

theogiraudet added the ✨ Module New modules label Nov 30, 2024

theogiraudet added this to the 3.0.0 milestone Nov 30, 2024

theogiraudet linked an issue Nov 30, 2024 that may be closed by this pull request

bs.string: A module to manage strings #191

Open

3 tasks

aure31 and others added 21 commits December 27, 2024 21:12

add the concat feature

8c69154

🐛 Fix move (mcbookshelf#289)

9cc7a31

🐛 Add missing blocks from latest Minecraft version in bs.block regist…

cb9862a

…ries (mcbookshelf#291)

🏷️ Dump to 2.2.1

2f43498

🐛 Fix creaking hitbox and improve tests to use packtest beta versions (…

5f64ab5

…mcbookshelf#294)

🏷️ Dump 2.2.2 (mcbookshelf#295)

335fd3d

Beet rework (mcbookshelf#285)

00cca8a

Update versions.json

44f2c45

Beet rework (mcbookshelf#285)

580983a

creation of bs.string

4db63f9

and implementation of the `index_of` and `lenth` feature

Merge branch 'master' into pr/283

555a606

Merge branch 'master' of https://github.com/aure31/Bookshelf into pr/283

d9077fd

merge + changing the concat with the advice of aksiom

309928a

+ starting the lower and upper case

to_number + to_string + lower_case + upper_case

2b26144

and come fix

starting the docs file

69e6fe9

find function

aed388e

fix some files and slighly modified find

b3ff5f1

fixes metadata

f7a1f02

maybe fixes the header

0199d4b

test + split + to list

405012d

fix bugs with the split, adding the replace function and more test : )

3d9ad42

aksiome reviewed Jan 17, 2025

View reviewed changes

aure31 and others added 3 commits January 17, 2025 15:45

fix of the aksiom review

8cc7109

🔖 Update generated metadata

d945eab

Merge branch 'mcbookshelf:master' into master

9896b61

🔖 Update generated metadata

ba5d1ac

aksiome force-pushed the master branch from fdcad35 to 7db34cb Compare January 24, 2025 20:11

aure31 and others added 9 commits January 25, 2025 19:24

ajout des differente pour benificié du cache

6e80ee4

Merge branch 'mcbookshelf:master' into master

3a03178

fix for concat

f98f2e5

add of the insert function

dbb9ef1

🔖 Update generated metadata

75c19ba

unsuccessful attempt to implement the precompute algo of the find fun…

236c908

…ction

fix find function

2a10868

disable the inverse search for the find function because of the need …

cca563d

…of the inverse table

fix find function

fa0236d

aksiome reviewed Jan 26, 2025

View reviewed changes

aure31 added 4 commits January 28, 2025 15:30

Merge branch 'mcbookshelf:master' into master

1c555e2

resolve all

8c85be3

adding the new algo to the split function

20bcfa0

make two row in one

e6625d6

aksiome changed the title ~~Add a new bs.string module~~ ✨ feat: add a new bs.string module Feb 26, 2025

aure31 and others added 6 commits February 27, 2025 21:52

fix bug + adding bs.log as weak depedencies

0521187

🔖 Update generated metadata

dc6bd3c

fix bugs and adding the algorithm to the replace function

ca135cc

adding reverse function and preventing corner case

c89efd4

avancement de la doc

567099a

doc fix + removed debug

1b6232e

aksiome marked this pull request as ready for review March 2, 2025 10:26

aksiome requested changes Mar 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ feat: add a new `bs.string` module #283

✨ feat: add a new `bs.string` module #283

aure31 commented Nov 30, 2024 •

edited

Loading

aure31 commented Nov 30, 2024

aksiome left a comment

aksiome left a comment

aksiome Jan 26, 2025

aure31 Jan 28, 2025

aksiome Jan 28, 2025

aksiome left a comment

aksiome Mar 2, 2025

aksiome Mar 2, 2025

aksiome Mar 2, 2025

aksiome Mar 2, 2025

aksiome Mar 2, 2025

aksiome Mar 2, 2025

aksiome Mar 2, 2025

aksiome Mar 2, 2025

aksiome Mar 2, 2025

aksiome Mar 2, 2025

✨ feat: add a new bs.string module #283

Are you sure you want to change the base?

✨ feat: add a new bs.string module #283

Conversation

aure31 commented Nov 30, 2024 • edited Loading

Tasks to do before merging

aure31 commented Nov 30, 2024

aksiome left a comment

Choose a reason for hiding this comment

aksiome left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aksiome left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

✨ feat: add a new `bs.string` module #283

✨ feat: add a new `bs.string` module #283

aure31 commented Nov 30, 2024 •

edited

Loading