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

Qute guide says parity properties are based on zero-based index, but they are based on one-based index #27931

Closed
fedinskiy opened this issue Sep 14, 2022 · 4 comments · Fixed by #27936
Labels
area/documentation area/qute The template engine kind/bug Something isn't working
Milestone

Comments

@fedinskiy
Copy link
Contributor

Describe the bug

According to the Qute guide[1], loops have several properties for accessing parity metadata, namely odd, even and indexParity. The guide says their output is based on zero-based "index"(as opposed to one-based "count").
Zero-based index means, that the first element has index 0 and the second has index 1, so the reader may assume, that the first element would be even(0 is an even number) at the second would be odd(1 is an odd number).
In practice, it works in more natural way, and first element is odd, second is even and so on.

[1] https://quarkus.io/guides/qute-reference#loop_section

Expected behavior

The guide should describe this parity as based on one-based index(or one-based count), preferably with examples.

Actual behavior

The guide describes behavior which is not only counterintuitive, but also different from the reality.

How to Reproduce?

Option one, use this template in your app:

{#for i in 10}
{#if i_isFirst}Here are some indexed numbers: {/if}
{i_index} (parity: {i_indexParity}){#if !i_isLast},{#else}.{/if}
{/for}
{#for i in 10}
{#if i_isFirst}Here are some counted numbers: {/if}
{i_count} (parity: {i_indexParity}){#if !i_isLast},{#else}.{/if}
{/for}

Option two, use this reproducer:
https://github.com/fedinskiy/reproducer/pull/new/reproducer/qute-even-are-odd

  1. Clone
  2. mvn quarkus:dev in the root folder
  3. Open http://localhost:8080/test

The result is the same:

Here are some indexed numbers: 0 (parity: odd), 1 (parity: even), 2 (parity: odd), 3 (parity: even), 4 (parity: odd), 5 (parity: even), 6 (parity: odd), 7 (parity: even), 8 (parity: odd), 9 (parity: even).
Here are some counted numbers: 1 (parity: odd), 2 (parity: even), 3 (parity: odd), 4 (parity: even), 5 (parity: odd), 6 (parity: even), 7 (parity: odd), 8 (parity: even), 9 (parity: odd), 10 (parity: even). 

It is easy to notice, that parity follows count and not index

Output of uname -a or ver

5.19.4-200.fc36.x86_64

Output of java -version

11.0.16 temurin

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.12.1.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)

Additional information

I assumed that the current implementation is right and the guide is wrong, since odd, even and indexParity properties are following the same convention, and this convention looks as more intuitive. If the guide is right and the implementation is wrong, then these three properties need to be changed, however this would be rather odd.

@fedinskiy fedinskiy added the kind/bug Something isn't working label Sep 14, 2022
@quarkus-bot quarkus-bot bot added the area/qute The template engine label Sep 14, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 14, 2022

/cc @mkouba

@mkouba
Copy link
Contributor

mkouba commented Sep 14, 2022

That's a bug. I think that we should keep the current behavior: (a) it's more intuitive and (b) we don't want to break compatibility. However, we'll need to fix the docs and also fix the code here, i.e. for the odd property we should return (count % 2 != 0) ? Results.TRUE : Results.FALSE. @fedinskiy would you care to send a pull request?

@fedinskiy
Copy link
Contributor Author

@mkouba I totally agree, that we should keep the current behavior. That is why I do not see any reason to "fix" the code, since even and odd work as expected.

I presume, you want to change the code use new variable "count", and not existing[1] field "index", but to keep its behavior intact, is that correct?

[1] https://github.com/quarkusio/quarkus/blob/main/independent-projects/qute/core/src/main/java/io/quarkus/qute/LoopSectionHelper.java#L257

@mkouba
Copy link
Contributor

mkouba commented Sep 14, 2022

I presume, you want to change the code use new variable "count", and not existing[1] field "index", but to keep its behavior intact, is that correct?

Exactly.

fedinskiy added a commit to fedinskiy/quarkus that referenced this issue Sep 14, 2022
See the issue[1] for context.
Also add a new test for this functionality.

[1] quarkusio#27931
fedinskiy added a commit to fedinskiy/quarkus that referenced this issue Sep 15, 2022
See the issue[1] for context.
Also add a new test for this functionality.

[1] quarkusio#27931
fedinskiy added a commit to fedinskiy/quarkus that referenced this issue Sep 15, 2022
See the issue[1] for context.
Also add a new test for this functionality.

[1] quarkusio#27931
Quarkus Documentation automation moved this from To do to Done Sep 16, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation area/qute The template engine kind/bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants