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

Add API for obtaining list of directories #20314

Merged
merged 1 commit into from Apr 29, 2024

Conversation

pktiuk
Copy link
Contributor

@pktiuk pktiuk commented Jan 22, 2024

This is just an early idea proposing adding an API which could be used for obtaining list of directories.

This could be used in every prompt in WEBUI which asks user about a specific path in the system. (It would be a really nice quality of life improvement)

Example implementation of mechanism helping with writing paths

obraz

(screenshot from syncthing)

I know this change would require also updating docs and is probably written badly. But now this is just a draft.

@luzpaz
Copy link
Contributor

luzpaz commented Jan 22, 2024

Commit has a typo in the title: dor -> for

@luzpaz luzpaz added the WebAPI WebAPI-related issues/changes label Jan 22, 2024
@pktiuk pktiuk changed the title Add API dor obtaining list of directories Add API for obtaining list of directories Jan 23, 2024
@pktiuk pktiuk changed the title Add API for obtaining list of directories Add API for autocompletion of directories Jan 23, 2024
@pktiuk pktiuk marked this pull request as ready for review January 23, 2024 19:45
@pktiuk
Copy link
Contributor Author

pktiuk commented Jan 23, 2024

Should I also update API version?

@pktiuk
Copy link
Contributor Author

pktiuk commented Jan 27, 2024

@glassez @thalieht

Could you look at this code?

@glassez
Copy link
Member

glassez commented Jan 28, 2024

@glassez @thalieht

Could you look at this code?

It would be more clear and robust if it just returns content of given directory (only subdirectories if we don't need files).
Also it should be named based on what it does (e.g., get directory listing), not what it could be used for in any particular case (autocompletion etc.).

@pktiuk
Copy link
Contributor Author

pktiuk commented Jan 28, 2024

@glassez

Also it should be named based on what it does (e.g., get directory listing), not what it could be used for

This is exactly why I used name autocompletion, because it completes provided paths. (not only lists directories)

It would be more clear and robust if it just returns content of given directory

Maybe it is a better idea. Filtering paths can be done on the frontend side and seems to be a bit more universal.

@glassez
Copy link
Member

glassez commented Jan 28, 2024

Filtering paths can be done on the frontend side and seems to be a bit more universal.

Exactly. It is enough to get the directory listing only once.

@pktiuk
Copy link
Contributor Author

pktiuk commented Jan 28, 2024

Is it good now?

@glassez
Copy link
Member

glassez commented Jan 28, 2024

Is it good now?

No.
I would expect that its parameter is the path to the target directory (i.e. the one whose list of subdirectories you want to get), and not to some of its file/subdirectory.

@pktiuk pktiuk force-pushed the directory_api branch 2 times, most recently from 084862f to 8cb5cf8 Compare January 28, 2024 17:57
@pktiuk

This comment was marked as outdated.

src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
@glassez glassez changed the title Add API for autocompletion of directories Add API dor obtaining list of directories Jan 29, 2024
@glassez
Copy link
Member

glassez commented Jan 29, 2024

@pktiuk
Are you the one who intends to add some functionality to WebUI that would use this API?

@pktiuk
Copy link
Contributor Author

pktiuk commented Jan 29, 2024

Yes, I want to implement it, but I would like deal with it later in separate PR.

AFAIK other WebUI-s may be also interested in this API (VueTorrent/VueTorrent#1459 ).

@luzpaz luzpaz changed the title Add API dor obtaining list of directories Add API for obtaining list of directories Jan 29, 2024
@pktiuk pktiuk requested a review from glassez January 30, 2024 21:38
@glassez glassez requested a review from Chocobo1 January 31, 2024 03:49
@Chocobo1
Copy link
Member

TBH I find setting up an environment for qBitTorrent really annoying and problematic (mainly because of the new QT, boost and libtorrent), so I created a simple Docker for development. Would you be interested in contribution with Dockerfile and simple instruction how to use it?

Although it is not for development, we already have something alike: https://github.com/qbittorrent/docker-qbittorrent-nox

Later it could be used for creating easy configs for devcontainers and would allow app development using Github Codespaces. It would make life for new developers much easier.

Speaking for myself:

  • devcontainers
    That probably would help other newcomers.
    However, I already have my own dev environment in Archlinux. Setting it up for qbt is dead easy and therefore I probably won't be maintaining devcontainers. You'll have to be sure that it won't rot through the test of time.
  • Github Codespaces
    I'm not interested.

src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
@pktiuk pktiuk force-pushed the directory_api branch 3 times, most recently from 54ed3a8 to e12e31a Compare February 16, 2024 09:09
@pktiuk pktiuk requested a review from glassez February 16, 2024 09:10
@pktiuk pktiuk force-pushed the directory_api branch 2 times, most recently from 29f43b3 to 6a8327c Compare February 16, 2024 09:35
src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
@pktiuk pktiuk force-pushed the directory_api branch 2 times, most recently from 5844b99 to c8fd032 Compare April 14, 2024 13:31
src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
Chocobo1
Chocobo1 previously approved these changes Apr 22, 2024
@glassez glassez added this to the 5.0 milestone Apr 29, 2024
@glassez glassez merged commit a1af077 into qbittorrent:master Apr 29, 2024
10 of 14 checks passed
@glassez
Copy link
Member

glassez commented Apr 29, 2024

@pktiuk
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebAPI WebAPI-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants