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

File tree diff client #409

Closed
stsewd opened this issue Oct 17, 2024 · 3 comments · Fixed by #424 or readthedocs/readthedocs.org#11749
Closed

File tree diff client #409

stsewd opened this issue Oct 17, 2024 · 3 comments · Fixed by #424 or readthedocs/readthedocs.org#11749
Assignees
Labels
Needed: design decision A core team decision is required

Comments

@stsewd
Copy link
Member

stsewd commented Oct 17, 2024

With readthedocs/readthedocs.org#11646, we are now exposing the file tree diff information to addons, but we still need to have a way to show the results to users.

What I'm thinking here is to have a small list of the changed files (the first 2 or 3 files of each category), and then have a "show all files" button that will open a popup with the list of the files, this list will have some options to filter the results (modified, added, deleted) and a search bar.

We can also expose a button that suggest how to create a redirect for files that were deleted.

@stsewd stsewd added the Needed: design decision A core team decision is required label Oct 17, 2024
@ericholscher
Copy link
Member

I think to start, even just outputting the data to the console, or as a <details> tag in the PR build warning seem like a good initial implantation. Just something where we can start surfacing the data to start playing with in dev.

@TheTripleV
Copy link

This is nice to have. We currently have a sphinx extension that does this for us with git. https://github.com/wpilibsuite/sphinxext-delta

It injects a new main toctree section that lists all the pages that have changed.

https://frc-docs--2680.org.readthedocs.build/en/2680/index.html

is an example of a PR build with the "pr changed files" toctree added.
We're currently fixing failures in this extension because RTD doesn't inject all the html context anymore.

@humitos
Copy link
Member

humitos commented Oct 30, 2024

I checked the API response returned at https://docs--11716.org.readthedocs.build/en/11716/

        "filetreediff": {
            "diff": {
                "added": [
                    {
                        "file": "guides/management/2fa.html"
                    }
                ],

It seems the file attribute is not a valid URL, but the path of the file on disk that changed. We will need to resolve all those paths into valid URLs in the backend and expose them to the frontend to generate the links from JavaScript. We can probably add a url attribute in the API response as well.

In this particular case, the URL we are looking for here would be /en/11716/guides/management/2fa.html.

@humitos humitos self-assigned this Nov 4, 2024
humitos added a commit that referenced this issue Nov 7, 2024
Added a new addon to show all the filenames and URLs that were added, deleted or
modified in the current PR compared to the LATEST version.

Closes #409
@github-project-automation github-project-automation bot moved this from Needs review to Done in 📍Roadmap Nov 12, 2024
humitos added a commit that referenced this issue Nov 12, 2024
Added a new addon to show all the filenames and URLs that were added,
deleted or modified in the current PR compared to the LATEST version.


![Screenshot_2024-11-07_12-44-02](https://github.com/user-attachments/assets/dfdba906-1eb1-4755-a1e6-eaddbfa51144)

I'm rendering this data in a new notification decoupled from the
existing one on purpose. I don't want to entangle the code of these two
different addons for now, in particular while we are under
development/testing. I also see that in the future they may have
different UI elements, or the data will be integrated in a more bigger
UI element (eg. toolbar) or similar.

We can discuss more how this addon will grow and where is a good place
to add this data and make that work in a following PR. For now, I want
something that we can start testing internally in our own projects.

### Examples


![Screenshot_2024-11-07_14-15-37](https://github.com/user-attachments/assets/d0265b34-8ade-4348-bc8e-e6a3ff8ae0d1)



![Screenshot_2024-11-07_14-35-41](https://github.com/user-attachments/assets/403b6860-60c1-468e-ae5d-2491f6f47b38)



![Screenshot_2024-11-07_14-51-06](https://github.com/user-attachments/assets/0e24664d-672f-40c0-9be4-bd76b2cf4f04)


Closes #409
Requires readthedocs/readthedocs.org#11749
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: design decision A core team decision is required
Projects
Archived in project
4 participants