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

\tl_case:nn, \str_case:Nn, and \str_if_empty:n #1071

Closed
lvjr opened this issue Mar 21, 2022 · 12 comments
Closed

\tl_case:nn, \str_case:Nn, and \str_if_empty:n #1071

lvjr opened this issue Mar 21, 2022 · 12 comments
Assignees
Labels
documentation Issues in the documenation enhancement New feature or request

Comments

@lvjr
Copy link
Contributor

lvjr commented Mar 21, 2022

Recently I compared tl and str packages:

  • there is \tl_case:Nn but not \str_case:Nn
  • there is \str_case:nn but not \tl_case:nn
  • there is \tl_if_empty:n but not \str_if_empty:n

Could these missing functions be added to expl3?

@eg9
Copy link
Contributor

eg9 commented Mar 21, 2022

There is \str_case:Vn which serves the same purpose as the proposed \str_case:Nn.

There is also an inherent difference between \str_case:Vn and \tl_case:Nn, as the latter compares token list variables (with \ifx) and not “explicit” token lists, so \tl_case:nn would be “strange”. It could be implemented by setting an internal variable to the given token list and then calling \tl_case:Nn for this variable, but I'm not sure this is something we should add.

Indeed, I find strange that \str_if_empty:n(TF) is missing.

@josephwright
Copy link
Member

I guess \str_if_empty:n was missed as it doesn't matter if one uses a tl function here - the nature of the non-space tokens is unimportant. However, it's easy enough to add.

As @eg9 says, \str_case:Nn is the same as \str_case:Vn: again easy to add and again I'm happy to.

I'm not keen on \tl_case:nn for the reasons outlined.

@lvjr
Copy link
Contributor Author

lvjr commented Mar 21, 2022

I think consistency is important. otherwise it will confuse users. In my opinion, it would be better to use \str_if_eq:NN inside \str_case:Nn function just like in tl package.

\str_set:Nn \l_tmpa_str {\abc}
\str_set:Nn \l_tmpb_str {\abc}
\str_case:VnF \l_tmpa_str
  {
    \uvw        { UVW }
    \l_tmpb_str { TMP }
    \abc        { ABC }
  }
  { No Match }

The above code gives ABC, which shows that \str_case:Vn in not \str_case:Nn.

@blefloch
Copy link
Member

I think \tl_case:Nn should be removed or renamed \cs_case_meaning:Nn. It is a leftover from the old days where one had to compare macros with stored values to get any reasonable speed.

@josephwright
Copy link
Member

I think @blefloch is right here: the approach of comparing token lists is actually not that useful in the main in my experience. We can't remove the name, but we can deprecate.

@josephwright
Copy link
Member

I think consistency is important. otherwise it will confuse users. In my opinion, it would be better to use \str_if_eq:NN inside \str_case:Nn function just like in tl package.

\str_set:Nn \l_tmpa_str {\abc}
\str_set:Nn \l_tmpb_str {\abc}
\str_case:VnF \l_tmpa_str
  {
    \uvw        { UVW }
    \l_tmpb_str { TMP }
    \abc        { ABC }
  }
  { No Match }

The above code gives ABC, which shows that \str_case:Vn in not \str_case:Nn.

I'm not sure what you mean here: the defined behaviour is that \str_case:Nn compares the value of each entry to a string. You've omitted the braces in your list of possible strings, but we do not expect to compare with the value of \l_tmpb_str, rather those tokens as a string.

@lvjr
Copy link
Contributor Author

lvjr commented Mar 21, 2022

I'm not sure what you mean here: the defined behaviour is that \str_case:Nn compares the value of each entry to a string. You've omitted the braces in your list of possible strings, but we do not expect to compare with the value of \l_tmpb_str, rather those tokens as a string.

Because \tl_case:Nn is a shorthand command for some nesting \tl_if_eq:NN, therefore in my mind \str_case:Nn should be a shorthand command for some nesting \str_if_eq:NN. That is to say, in my mind the syntax of \str_case:Nn is

\str_case:Nn <test str variable> {
  <str variable case 1> {<code case 1>}
  <str variable case 2> {<code case 2>}
  <str variable case 3> {<code case 3>}
}

@josephwright josephwright self-assigned this Jun 24, 2022
@josephwright josephwright added expl3 enhancement New feature or request documentation Issues in the documenation labels Jun 24, 2022
@blefloch
Copy link
Member

blefloch commented Jun 24, 2022

We concluded \tl_case:Nn should be deprecated and renamed \cs_case_meaning:Nn, but there is already \token_case_meaning:Nn, which seems to fit the bill. Relatedly, \tl_if_eq:NN(TF), \cs_if_eq:NN(TF) and \token_if_eq_meaning:NN(TF) have identical definitions, and probably some should be defined as copies of others with the appropriate prg_new_eq function.

@josephwright
Copy link
Member

We concluded \tl_case:Nn should be deprecated and renamed \cs_case_meaning:Nn, but there is already \token_case_meaning:Nn, which seems to fit the bill. Relatedly, \tl_if_eq:NN(TF), \cs_if_eq:NN(TF) and \token_if_eq_meaning:NN(TF) have identical definitions, and probably some should be defined as copies of others with the appropriate prg_new_eq function.

Multiple names are fine as long as they help with understanding: I think \tl_if_eq:NN and \cs_if_eq:NN certainly fit that bill. I'll look at the tidy-ups shortly.

@josephwright
Copy link
Member

We still need some action here?

josephwright added a commit that referenced this issue May 23, 2023
josephwright added a commit that referenced this issue May 23, 2023
This closes #1071: all tasks done.
@muzimuzhi
Copy link
Contributor

muzimuzhi commented May 23, 2023

I guess \str_if_empty:n was missed as it doesn't matter if one uses a tl function here - the nature of the non-space tokens is unimportant. However, it's easy enough to add.

As @eg9 says, \str_case:Nn is the same as \str_case:Vn: again easy to add and again I'm happy to.

quoted from #1071 (comment)

Still planning to add \str_if_empty:nTF and \str_case:Vn?

FYI: They were added in 025bde6 and 600f6c3, respectively.

@josephwright
Copy link
Member

I guess \str_if_empty:n was missed as it doesn't matter if one uses a tl function here - the nature of the non-space tokens is unimportant. However, it's easy enough to add.
As @eg9 says, \str_case:Nn is the same as \str_case:Vn: again easy to add and again I'm happy to.
quoted from #1071 (comment)

Still planning to add \str_if_empty:nTF and \str_case:Vn?

They were done the same day they were discussed here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues in the documenation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants