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

HTML column sorting is inconsistent across index pages #1766

Closed
nedbat opened this issue Apr 24, 2024 · 4 comments · Fixed by #1768
Closed

HTML column sorting is inconsistent across index pages #1766

nedbat opened this issue Apr 24, 2024 · 4 comments · Fixed by #1768
Labels
bug Something isn't working html

Comments

@nedbat
Copy link
Owner

nedbat commented Apr 24, 2024

Describe the bug

  1. Visit the sample HTML report: https://nedbatchelder.com/files/sample_coverage_html/index.html
  2. Click the "coverage" column to sort by total coverage percentage
  3. Click the "Functions" tab at the top to switch to the by-function index page.
  4. Now the "partial" column is sorted.

This is because the number of the column is stored, but the function and class index pages have one more column, so the numbering is off.

This is with coverage.py 7.5.0.

Expected behavior
If you've sorted by a column, the same-named column should be sorted when switching to another page.

@nedbat nedbat added bug Something isn't working needs triage html and removed needs triage labels Apr 24, 2024
@devdanzin
Copy link
Contributor

Seems like the simple way to do it would be adding ids the the headers, getting the th from (and storing) it in sortColumn and loading it in coverage.wire_up_sorting (which would also pass the id from the click event).

I can work on a PR tonight if it's still open.

@nedbat
Copy link
Owner Author

nedbat commented Apr 24, 2024

Thanks! Extra-credit homework: if you sort the function page by function name, the file index page doesn't have that column, so what should the sort order be?

@devdanzin
Copy link
Contributor

We could store the last column the index page was sorted by and sort on it again. But then we'd have one more question when the user goes back to the function page. We have two scenarios:

  • The user didn't re-sort by another column: should the function page now be sorted by function (as it was last time the user was there) or by whatever was the last sort column on the index page?
  • The user sorted by some other column: then we can sort the function/class page by that new column.

I think it would be easier to keep storing the last column to sort with, while adding a flag that means "sort the function or class pages by function/class" when set. Let me give it a try.

nedbat pushed a commit that referenced this issue Apr 30, 2024
…#1766) (#1768)

* Add ids to column headers that you can sort by.

* Use table header ids to remember sort columns, special casing the function/class column.

* Rename 'function_or_class' and related names to 'region' and related.

* Use 'file' as placeholder for the th id, so we sort on file in the absence of a recorded column id.

* Fix bug of incorrectly recording direction changes.

* Use 'let' instead of 'var' in the new code.

* Update gold HTML tests.

* Support scrubbing temporary paths in test_html.py on Windows.

* Fix long line in test_html.

* Add gold files for test_html partial (needs running tests on Python < 3.10).

* Use scrub for Windows path when running test suite in all OSes.

* Move scrubbing of Windows paths into test_other.
@nedbat
Copy link
Owner Author

nedbat commented May 4, 2024

This is now released as part of coverage 7.5.1.

renovate bot added a commit to allenporter/flux-local that referenced this issue May 4, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [coverage](https://togithub.com/nedbat/coveragepy) | `==7.5.0` ->
`==7.5.1` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/coverage/7.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/coverage/7.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/coverage/7.5.0/7.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/coverage/7.5.0/7.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>nedbat/coveragepy (coverage)</summary>

###
[`v7.5.1`](https://togithub.com/nedbat/coveragepy/blob/HEAD/CHANGES.rst#Version-751--2024-05-04)

[Compare
Source](https://togithub.com/nedbat/coveragepy/compare/7.5.0...7.5.1)

- Fix: a pragma comment on the continuation lines of a multi-line
statement
now excludes the statement and its body, the same as if the pragma is
on the first line. This closes `issue 754`*. The fix was contributed by
    `Daniel Diniz <pull 1773_>`*.

- Fix: very complex source files like `this one <resolvent_lookup_>`\_
could
cause a maximum recursion error when creating an HTML report. This is
now
    fixed, closing `issue 1774`\_.

-   HTML report improvements:

- Support files (JavaScript and CSS) referenced by the HTML report now
have
hashes added to their names to ensure updated files are used instead of
        stale cached copies.

- Missing branch coverage explanations that said "the condition was
never
false" now read "the condition was always true" because it's easier to
        understand.

- Column sort order is remembered better as you move between the index
pages,
        fixing `issue 1766`*.  Thanks, `Daniel Diniz <pull 1768_>`*.

.. \_resolvent_lookup:
https://github.com/sympy/sympy/blob/130950f3e6b3f97fcc17f4599ac08f70fdd2e9d4/sympy/polys/numberfields/resolvent_lookup.py
.. \_issue
754[nedbat/coveragepy#754
.. \_issue
176[nedbat/coveragepy#1766
.. \_pull
17[nedbat/coveragepy#1768
.. \_pull
1[nedbat/coveragepy#1773
.. \_issue
[nedbat/coveragepy#1774

.. \_changes\_7-5-0:

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMzEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjMzMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working html
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants