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

length: Returns the length of the given string/list/map #24

Merged

Conversation

chucker
Copy link
Collaborator

@chucker chucker commented Aug 12, 2023

Only for strings so far.

I'm not sure how to create maps and lists; perhaps by parsing a piece of code?

  • support strings
  • support maps
  • support lists
    - [ ] properly implement trimming (IL2026, IL2072) see trimmable reflection #28

@ricardoboss
Copy link
Owner

I like the automatic registration of native functions. I wouldn't classify it as IO, though. Either move it to "Conversion" (since it returns a different type) or create a new one.

if (exp is not ConstantExpression conExp)

will throw for everything but constants directly passed to the function. After getting the single expression, you need to evaluate it using the interpreter. Afterwards, you can work with the ExpressionResult.

@chucker
Copy link
Collaborator Author

chucker commented Aug 13, 2023

I wouldn't classify it as IO, though.

I originally had a new group (String?), but the polymorphic nature makes it hard to classify.

@ricardoboss
Copy link
Owner

What about Conversion as per the proposed categories for all new functions?

@chucker
Copy link
Collaborator Author

chucker commented Aug 15, 2023

dotnet/sdk#27997 (comment)

I'm not sure this rabbit hole is worth it.

@chucker chucker force-pushed the features/implement-builtin-function branch from 634dbc8 to 2cfdd34 Compare August 15, 2023 19:02
@chucker
Copy link
Collaborator Author

chucker commented Aug 15, 2023

The automatic registration is now part of #28. We either fix trimming, or remove that requirement; either way, it isn't really directly related to this PR.

@chucker
Copy link
Collaborator Author

chucker commented Aug 15, 2023

What about Conversion as per the proposed categories for all new functions?

I don't love it, but IO isn't great either. Done.

@chucker chucker marked this pull request as ready for review August 15, 2023 20:01
@ricardoboss ricardoboss force-pushed the features/implement-builtin-function branch from a3eacec to 7ef436a Compare August 19, 2023 18:16
StepLang.Tests/Framework/IO/LengthFunctionTest.cs Outdated Show resolved Hide resolved
StepLang.Tests/Framework/IO/LengthFunctionTest.cs Outdated Show resolved Hide resolved
StepLang/Framework/Conversion/LengthFunction.cs Outdated Show resolved Hide resolved
@ricardoboss ricardoboss mentioned this pull request Aug 25, 2023
19 tasks
@ricardoboss
Copy link
Owner

@chucker I added a section about how the new functions should be documented. Please add some documentation for the length function at StepLang.Wiki/Functions/Length.md with the sections "Description", "Syntax"/"Synopsis", "Remarks" and "Examples".

For reference, you can look at the substring function documentation at https://github.com/ricardoboss/STEP/blob/main/StepLang.Wiki/Functions/Substring.md

@ricardoboss ricardoboss changed the title length function length: Returns the length of the given string/list/map Sep 1, 2023
@ricardoboss ricardoboss force-pushed the features/implement-builtin-function branch from abede58 to 5ff08b1 Compare September 8, 2023 15:31
@ricardoboss ricardoboss force-pushed the features/implement-builtin-function branch from 2f2f5b0 to 0325402 Compare September 8, 2023 19:52
@ricardoboss ricardoboss merged commit 5450037 into ricardoboss:main Sep 10, 2023
10 checks passed
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.

2 participants