Skip to content

Conversation

H1nr1
Copy link
Contributor

@H1nr1 H1nr1 commented Feb 11, 2025

This PR adds to the description of the in and inFold functions of custom command reference to specify the first argument of the functions can be a slice or string.
This PR also reorders the mentioned functions so that inFold follows in, which both allows inFold's description to base off of in's description and follows the order as seen with the reFind family of functions.

Terms

@SoggySaussages
Copy link
Contributor

SoggySaussages commented Feb 11, 2025

What do you think about changing list to container?
Then we’re not calling a variable which could possibly
be a string a “list.”

@H1nr1
Copy link
Contributor Author

H1nr1 commented Feb 11, 2025

I'm not sure of any other instances of "container" to describe an argument that can vary in type. I kept "list" as that seems to be what other functions use in similar cases. While list may have originally represented the type slice, it could also be interpreted as "a list of types."

I don't know of a correct way to go about this. If we were to change the terminology used to describe varying type arguments, then many other instances of "list"; which isn't a big deal, just putting it out there as a reminder.

@H1nr1
Copy link
Contributor Author

H1nr1 commented Feb 11, 2025

I thought of changing "list" to "value" and "value" to "subvalue", but didn't want to change too much in one go.

With this, syntax for in would be presented as {{$result := in <value> <subvalue>}}

I think this would be quite easy to explain, but the double instance of "value" could throw some people off.

@galen8183
Copy link
Contributor

I think list is fine, but maybe sequence would read better to you?

@H1nr1
Copy link
Contributor Author

H1nr1 commented Feb 11, 2025

I think we stick with list so far. Long shot, but sequence could possibly confuse readers with the sequence function.

@SoggySaussages
Copy link
Contributor

I'm not sure of any other instances of "container" to describe an argument that can vary in type. I kept "list" as that seems to be what other functions use in similar cases. While list may have originally represented the type slice, it could also be interpreted as "a list of types."

I don't know of a correct way to go about this. If we were to change the terminology used to describe varying type arguments, then many other instances of "list"; which isn't a big deal, just putting it out there as a reminder.

Which other instances of list are you thinking of?

To me, it seems inaccurate to describe it as a list, as if
we say that a string is actually a list of sub strings, and
value is one of the items in this list, that doesn’t seem
like an accurate way to describe a string.

E.x. in "yagpdb" "gpd". If we say yagpdb is the list ,
we’re kinda implying that it’s a list that contains "ya"
"gpd" and "b," which doesn’t sit right with me. I think it’s
one thing if we are saying that a string could be a list of
characters, if we are only searching for one character at
any given time, but saying that it’s actually a list of sub
strings, which are arbitrarily sliced out of the original
string, that doesn’t seem logically accurate.

I think it’s logically accurate to say a list contains items,
and a string contains a sub string. What both of these
have in common is the ability to contain, and therefore I think it would be accurate to call them each containers.

Could also maybe consider needle and haystack,
although I would argue that has the same problems as
list and value.

@SoggySaussages
Copy link
Contributor

I thought of changing "list" to "value" and "value" to "subvalue", but didn't want to change too much in one go.

With this, syntax for in would be presented as {{$result := in <value> <subvalue>}}

I think this would be quite easy to explain, but the double instance of "value" could throw some people off.

Ha ha I also considered that exactly but discarded it too

@SoggySaussages
Copy link
Contributor

I think list is fine, but maybe sequence would read better to you?

I think sequence does read better to me than list, yeah.

@H1nr1
Copy link
Contributor Author

H1nr1 commented Feb 11, 2025

First instance I noticed of list used in place of an argument that can vary in type is index, just below in in the docs. This time, list can be a slice, map, or string; so even more variety. I think covering these types with keywords will take a lot of communication to agree upon a term that fits for each set of types.

I didn't see the relation you made when suggesting container at first. With in being the equivalent to a contains function in many other languages, it makes sense. But again, having specific terms for each set of accepted types can become confusing; the description of each function can handle the listing of accepted types for each argument, in my opinion.

@H1nr1
Copy link
Contributor Author

H1nr1 commented Feb 11, 2025

Are we saying sequence as in a sequence of values or characters, to represent both slices and strings, respectively?

@SoggySaussages
Copy link
Contributor

First instance I noticed of list used in place of an argument that can vary in type is index, just below in in the docs. This time, list can be a slice, map, or string; so even more variety. I think covering these types with keywords will take a lot of communication to agree upon a term that fits for each set of types.

I didn't see the relation you made when suggesting container at first. With in being the equivalent to a contains function in many other languages, it makes sense. But again, having specific terms for each set of accepted types can become confusing; the description of each function can handle the listing of accepted types for each argument, in my opinion.

I think index differs in that it is accurate to say a string is
a list of characters, of which we are retrieving one single
character from. The logic gets messy when you apply
that same logic to the end function, see my above
comment and the “yagpdb” example.

@galen8183
Copy link
Contributor

galen8183 commented Feb 11, 2025

Are we saying sequence as in a sequence of values or characters, to represent both slices and strings, respectively?

Yes, it's a more technically correct description of strings than list too.

I think "Returns whether the [string] is in the sequence", substituting 'value', makes sense both for strings and slices.

@H1nr1
Copy link
Contributor Author

H1nr1 commented Feb 11, 2025

Ok, yes, that makes more sense. I will make that change now.

@H1nr1
Copy link
Contributor Author

H1nr1 commented Feb 11, 2025

Personally, I'm not a fan of "Same as in." What are y'all's thoughts on instead copying in's description to inFold but changing to say case-insensitive?

Copy link
Collaborator

@l-zeuch l-zeuch left a comment

Choose a reason for hiding this comment

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

(Unfortunately I haven't yet set up the lint workflow to add comments by itself--this is its output on your modified files)

@l-zeuch
Copy link
Collaborator

l-zeuch commented Feb 14, 2025

Thanks for the PR (and sorry for the very late actual response).

It is my understanding that all voiced concerns have been addressed—is that the case?
I'd prefer to keep the "same as [in]" line, not only because my personal goal is to be DRY, but also to make it very clear that these functions differ only in case-sensitivity.

I'll mark this as approved for now and merge it ~this weekend, given no new details pop up.

@H1nr1
Copy link
Contributor Author

H1nr1 commented Feb 14, 2025

In that case, all is good. Thanks!

@l-zeuch l-zeuch merged commit a82009e into botlabs-gg:master Feb 15, 2025
2 of 3 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.

4 participants