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

rustdoc search: Allow to filter on multiple crates #115802

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Sep 12, 2023

This is something that was discussed some time ago (can't find where though...) so I finally took some time to write a first draft.

Motivation

As a user of a group of crates with FFI counterpart (like gtk-rs, docs available here), I want to not only be able to pick one crate to run the search on but instead pick multiple crates that I want to exclude so that I can exclude results from FFI crates.

Adding this feature allows to decide not only which crate you want to include but also which crates you want to exclude from search results.

Another example is: you're using bevy and a lot of additional crates to add more functionalities (like bevy_egui) and you want to look at how to get cursor position in both bevy and bevy_egui without all other crates. This feature allows to do it in one search as well.

Implementation details

First I tried to use HTML elements to do so (multiple attribute on select in particular) however I found it really bad as you have to maintain ctrl (or cmd on mac) and click on multiple lines to be able to select more than one element, which isn't great.

So since I couldn't find a native way to do this, I re-used the popover code we already have for the other menus. To prevent reloading every time you check/uncheck a box, I decided to add a "refresh" button instead which is disabled until you pick a different filtering selection.

You can give it a try here.

Unresolved questions

In case there are a lot of crates and you want to only pick one, you'll have to unselect all others, which is quite bad. I had the idea when you checked/unchecked a box to make a small button appear right below it for a few seconds which would allow to do that. Not sure if it's a good idea or not though.

Remaining to be done

Since this is a draft, this PR isn't complete, here are the missing parts:

  • Set a maximum height on this filter menu and add overflow-y: auto to prevent it taking too much height when you have too many crates.
  • Move the settings.css content back into main.css (I plan to do this in any case as there isn't much point for this file to exist considering it contains very few CSS rules).
  • Add GUI tests
  • Add JS search tests
  • Make an FCP and write the complete description with screenshots once the UI is defined.

What do you think about it?

cc @jsha
r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 12, 2023
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v3' (SHA:f43a0e5ff2bd294095638e18286ca9a3d1956744)
Download action repository 'actions/upload-artifact@v3' (SHA:a8a3f3ad30e3422c9c7b888a15615d19a852ae32)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
Removing intermediate container 6aab0d16cc4e
 ---> 0b460fd4af8b
Step 6/10 : COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
 ---> 9c2c36c00af2
Step 7/10 : RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt     && pip3 install virtualenv
Collecting binaryornot==0.4.4
  Downloading binaryornot-0.4.4-py2.py3-none-any.whl (9.0 kB)
Collecting boolean-py==4.0
  Downloading boolean.py-4.0-py3-none-any.whl (25 kB)
---
Building wheels for collected packages: reuse
  Building wheel for reuse (pyproject.toml): started
  Building wheel for reuse (pyproject.toml): finished with status 'done'
  Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=180117 sha256=2196c9034bf565528bbb1ee6dad4f753eb813f58822363e6b768f09c73e4d4ff
  Stored in directory: /tmp/pip-ephem-wheel-cache-ekn4kyhj/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d
Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet
  Attempting uninstall: setuptools
    Found existing installation: setuptools 59.6.0
    Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
    Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
    Can't uninstall 'setuptools'. No files were found to uninstall.
Successfully installed binaryornot-0.4.4 boolean-py-4.0 chardet-5.1.0 jinja2-3.1.2 license-expression-30.0.0 markupsafe-2.1.1 python-debian-0.1.49 reuse-1.1.0 setuptools-66.0.0
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
Collecting virtualenv
  Downloading virtualenv-20.24.5-py3-none-any.whl (3.7 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.7/3.7 MB 17.8 MB/s eta 0:00:00
Collecting distlib<1,>=0.3.7
  Downloading distlib-0.3.7-py2.py3-none-any.whl (468 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 468.9/468.9 KB 50.3 MB/s eta 0:00:00
Collecting platformdirs<4,>=3.9.1
  Downloading platformdirs-3.10.0-py3-none-any.whl (17 kB)
Collecting filelock<4,>=3.12.2
Collecting typing-extensions>=4.7.1
  Downloading typing_extensions-4.7.1-py3-none-any.whl (33 kB)
  Downloading typing_extensions-4.7.1-py3-none-any.whl (33 kB)
Installing collected packages: distlib, typing-extensions, platformdirs, filelock, virtualenv
Successfully installed distlib-0.3.7 filelock-3.12.3 platformdirs-3.10.0 typing-extensions-4.7.1 virtualenv-20.24.5
Removing intermediate container 9468a840c66a
 ---> 32f879311a91
Step 8/10 : COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
 ---> dcf4b4488e34
 ---> dcf4b4488e34
Step 9/10 : COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/
 ---> 35bc555de734
Step 10/10 : ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
Removing intermediate container ef26e0b21058
 ---> bff981dc6ec6
Successfully built bff981dc6ec6
Successfully tagged rust-ci:latest
Successfully tagged rust-ci:latest
##[endgroup]
Built container sha256:bff981dc6ec656aa0a496151015455589760596b0a9b37d3503c334f04d46443
Uploading finished image sha256:bff981dc6ec656aa0a496151015455589760596b0a9b37d3503c334f04d46443 to https://ci-caches.rust-lang.org/docker/31024a61e3c099341c4151411b61536c5a15e77991c1c58c8adf475bd61d8568f58aad59ac3149cca672919d01d664a203adb87f86665b006743ece22accf3f1
IMAGE          CREATED              CREATED BY                                      SIZE      COMMENT
bff981dc6ec6   1 second ago         /bin/sh -c #(nop)  ENV SCRIPT=TIDY_PRINT_DIF…   0B        
dcf4b4488e34   2 seconds ago        /bin/sh -c #(nop) COPY file:078ea1d11e7b7cda…   367B      
32f879311a91   3 seconds ago        |1 DEBIAN_FRONTEND=noninteractive /bin/sh -c…   24.2MB    
9c2c36c00af2   10 seconds ago       /bin/sh -c #(nop) COPY file:ac591dd6bc5afa66…   5.33kB    
0b460fd4af8b   11 seconds ago       |1 DEBIAN_FRONTEND=noninteractive /bin/sh -c…   23.1MB    
---
<missing>      3 weeks ago          /bin/sh -c #(nop)  LABEL org.opencontainers.…   0B        
<missing>      3 weeks ago          /bin/sh -c #(nop)  ARG LAUNCHPAD_BUILD_ARCH     0B        
<missing>      3 weeks ago          /bin/sh -c #(nop)  ARG RELEASE                  0B        

<botocore.awsrequest.AWSRequest object at 0x7f27c5619d10>
gzip: stdout: Broken pipe
xargs: docker: terminated by signal 13
https://ci-caches.rust-lang.org/docker/31024a61e3c099341c4151411b61536c5a15e77991c1c58c8adf475bd61d8568f58aad59ac3149cca672919d01d664a203adb87f86665b006743ece22accf3f1
sha256:bff981dc6ec656aa0a496151015455589760596b0a9b37d3503c334f04d46443
---
DirectMap4k:      202688 kB
DirectMap2M:     8185856 kB
DirectMap1G:    10485760 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
    Finished dev [unoptimized] target(s) in 0.03s
##[endgroup]
downloading https://ci-artifacts.rust-lang.org/rustc-builds-alt/e5fedceabf4e0564231db592b6d1f35e1ca27908/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz
extracting /checkout/obj/build/cache/llvm-e5fedceabf4e0564231db592b6d1f35e1ca27908-true/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz to /checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm
---
##[endgroup]
fmt check
tidy check
tidy: Skipping binary file check, read-only filesystem
##[error]tidy error: /checkout/src/librustdoc/html/static/js/search.js:1858: line longer than 100 chars
removing old virtual environment
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10'
Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (23.2.1)
Collecting black==23.3.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 7))
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 17.8 MB/s eta 0:00:00
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 17.8 MB/s eta 0:00:00
Collecting click==8.1.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 34))
  Downloading click-8.1.3-py3-none-any.whl (96 kB)
Collecting importlib-metadata==6.7.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 38))
  Downloading importlib_metadata-6.7.0-py3-none-any.whl (22 kB)
  Downloading importlib_metadata-6.7.0-py3-none-any.whl (22 kB)
Collecting mypy-extensions==1.0.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 42))
  Downloading mypy_extensions-1.0.0-py3-none-any.whl (4.7 kB)
Collecting packaging==23.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 46))
  Downloading packaging-23.1-py3-none-any.whl (48 kB)
Collecting pathspec==0.11.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 50))
  Downloading pathspec-0.11.1-py3-none-any.whl (29 kB)
  Downloading pathspec-0.11.1-py3-none-any.whl (29 kB)
Collecting platformdirs==3.6.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 54))
  Downloading platformdirs-3.6.0-py3-none-any.whl (16 kB)
Collecting ruff==0.0.272 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 58))
  Downloading ruff-0.0.272-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (5.9 MB)
Collecting tomli==2.0.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 77))
  Downloading tomli-2.0.1-py3-none-any.whl (12 kB)
Collecting typed-ast==1.5.4 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 81))
  Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB)
  Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 877.7/877.7 kB 76.1 MB/s eta 0:00:00
Collecting typing-extensions==4.6.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 107))
  Downloading typing_extensions-4.6.3-py3-none-any.whl (31 kB)
Collecting zipp==3.15.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 114))
  Downloading zipp-3.15.0-py3-none-any.whl (6.8 kB)
Installing collected packages: zipp, typing-extensions, typed-ast, tomli, ruff, platformdirs, pathspec, packaging, mypy-extensions, click, importlib-metadata, black
Successfully installed black-23.3.0 click-8.1.3 importlib-metadata-6.7.0 mypy-extensions-1.0.0 packaging-23.1 pathspec-0.11.1 platformdirs-3.6.0 ruff-0.0.272 tomli-2.0.1 typed-ast-1.5.4 typing-extensions-4.6.3 zipp-3.15.0
some tidy checks failed
Build completed unsuccessfully in 0:00:49
  local time: Tue Sep 12 21:16:18 UTC 2023
  network time: Tue, 12 Sep 2023 21:16:18 GMT

@bors
Copy link
Contributor

bors commented Sep 13, 2023

☔ The latest upstream changes (presumably #115820) made this pull request unmergeable. Please resolve the merge conflicts.

@notriddle
Copy link
Contributor

The design of this control makes it easier to search multiple crates at once, at the cost of making it more tedious to switch from searching one crate to searching another. I'm not sure that's true.

#99469 implements a more comprehensive tool for advanced searches. I think this feature makes more sense as part of that form.

@GuillaumeGomez
Copy link
Member Author

The design of this control makes it easier to search multiple crates at once, at the cost of making it more tedious to switch from searching one crate to searching another. I'm not sure that's true.

Absolutely. I have a few ideas for improvements which I listed but I'm not sure if the end result will still be good.

#99469 implements a more comprehensive tool for advanced searches. I think this feature makes more sense as part of that form.

For this one, we'll definitely need a designer or at least someone who can make a sketch that could be implemented because this is really bad currently.

@Dylan-DPC Dylan-DPC added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants