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 ability to display torrent "privateness" in UI #20951

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

Conversation

ManiMatter
Copy link
Contributor

@ManiMatter ManiMatter commented Jun 14, 2024

Adds private flag to GUI that is available in /info and /properties response

  • WebUI & GUI: +"Private" column in torrent list (hidden by default)
  • WebUI & GUI: +"Private" flag in properties of selected torrent ("Information" section)

@ManiMatter ManiMatter marked this pull request as draft June 14, 2024 10:43
@ManiMatter

This comment was marked as resolved.

@ManiMatter ManiMatter marked this pull request as ready for review June 14, 2024 16:33
@thalieht
Copy link
Contributor

On the GUI, sorting by the "Private" column does not work.

You have to add the column to TransferlistModel::InternalValue

Problem in General tab in GUI only:

  • Select a private torrent
  • Select a torrent without metadata
  • Private field in General tab is not updated to "No"

src/gui/transferlistmodel.cpp Outdated Show resolved Hide resolved
src/gui/transferlistmodel.cpp Outdated Show resolved Hide resolved
src/gui/transferlistmodel.cpp Outdated Show resolved Hide resolved
@ManiMatter

This comment was marked as outdated.

@thalieht
Copy link
Contributor

What's the best way to generate a torrent without metadata?

Add a magnet link to session and pause it before its metadata are retrieved.

@ManiMatter ManiMatter force-pushed the add-private-flag-to-GUI branch 2 times, most recently from 532d603 to 42a56f6 Compare June 15, 2024 06:56
src/gui/transferlistmodel.cpp Outdated Show resolved Hide resolved
src/gui/properties/propertieswidget.cpp Outdated Show resolved Hide resolved
src/gui/transferlistsortmodel.cpp Outdated Show resolved Hide resolved
src/webui/www/private/scripts/prop-general.js Outdated Show resolved Hide resolved
src/webui/www/private/scripts/dynamicTable.js Outdated Show resolved Hide resolved
src/webui/www/private/scripts/dynamicTable.js Outdated Show resolved Hide resolved
src/webui/www/private/scripts/dynamicTable.js Outdated Show resolved Hide resolved
@ManiMatter

This comment was marked as outdated.

@stalkerok
Copy link
Contributor

What's the best way to generate a torrent without metadata?

Add a magnet link to session and pause it before its metadata are retrieved.

This can also be done by adding a non-existent magnet link or you can use an existing one by changing a few characters.

@ManiMatter

This comment was marked as outdated.

@ManiMatter

This comment was marked as resolved.

@thalieht
Copy link
Contributor

Anyone? Already a pointer where to look to find a solution would be appreciated. I‘m tapping in the dark as to why this happens..

This will take care of it: #20951 (comment)

@glassez glassez changed the title Add private flag to GUI Add ability to display torrent "privateness" in UI Jun 22, 2024
@glassez glassez added WebUI WebUI-related issues/changes GUI GUI-related issues/changes labels Jun 22, 2024
@ManiMatter ManiMatter force-pushed the add-private-flag-to-GUI branch 2 times, most recently from bb61f0e to ec5d3a7 Compare June 22, 2024 14:47
@ManiMatter

This comment was marked as resolved.

@ManiMatter
Copy link
Contributor Author

@stalkerok - is the Windows version still crashing after the additional changes from @Chocobo1 ?
If yes, are there any logs or anything that helps us understand why that might be the case?

@stalkerok

This comment was marked as outdated.

@ManiMatter

This comment was marked as resolved.

@stalkerok

This comment was marked as off-topic.

@ManiMatter

This comment was marked as outdated.

@xavier2k6
Copy link
Member

Exception Information
MSVCP140!MTX_DO_LOCK+9CIn qbittorrent.exe.6044.dmp the assembly instruction at msvcp140!mtx_do_lock+9c in C:\Windows\System32\msvcp140.dll from Microsoft Corporation has caused an access violation exception (0xC0000005) when trying to read from memory location 0x0000[0](https://github.com/qbittorrent/qBittorrent/pull/20951#B1C1Thread3104)000 on thread 0

@stalkerok Can you grab/install latest Microsoft Visual C++ Redistributable & see if crash still happens.
Microsoft Visual C++ Redistributable Latest Supported Downloads

src/gui/transferlistmodel.cpp Outdated Show resolved Hide resolved
src/gui/transferlistmodel.cpp Outdated Show resolved Hide resolved
@@ -512,6 +525,8 @@ QVariant TransferListModel::internalValue(const BitTorrent::Torrent *torrent, co
return QVariant::fromValue(torrent->infoHash().v2());
case TR_REANNOUNCE:
return torrent->nextAnnounce();
case TR_PRIVATE:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate value corresponding to "n/a" state should be provided in order to sort by "Private" column properly. I believe -1 could do the job.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - would this work?4595dda

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - would this work?4595dda

No.
Did I suggest returning its string representation?
Isn't it obvious that internalValue() should provide something else (except, of course, the cases when the data itself is string)?
IMO, I made it clear that it is only necessary to add some third value that would correspond to the case when the "privateness" is not available (due to the lack of metadata).

@stalkerok

This comment was marked as off-topic.

@thalieht

This comment was marked as off-topic.

@stalkerok

This comment was marked as off-topic.

@xavier2k6

This comment was marked as off-topic.

@ManiMatter
Copy link
Contributor Author

Shall we tag this to milestone 5.0?

@glassez
Copy link
Member

glassez commented Jun 30, 2024

Shall we tag this to milestone 5.0?

I don't think this is a good idea for a PR that doesn't have a predictable completion date. It may not make it to v5.0 in time.

@ManiMatter
Copy link
Contributor Author

@glassez - I was under the impression that this one is pretty much done. Do you see remaining items that need to be addressed?

@glassez
Copy link
Member

glassez commented Jun 30, 2024

@glassez - I was under the impression that this one is pretty much done. Do you see remaining items that need to be addressed?

Yes. At least #20951 (comment).

@ManiMatter ManiMatter force-pushed the add-private-flag-to-GUI branch 3 times, most recently from d803edd to d60c77c Compare June 30, 2024 13:42
src/gui/properties/propertieswidget.cpp Outdated Show resolved Hide resolved
src/gui/properties/propertieswidget.cpp Outdated Show resolved Hide resolved
src/gui/transferlistmodel.cpp Outdated Show resolved Hide resolved
+Changes from code review
@@ -199,6 +199,7 @@ int TransferListSortModel::compare(const QModelIndex &left, const QModelIndex &r
case TransferListModel::TR_POPULARITY:
return customCompare(leftValue.toReal(), rightValue.toReal());

case TransferListModel::TR_PRIVATE:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to add the following function:

    int customCompare(const QVariant &left, const QVariant &right)
    {
        const bool isLeftValid = left.isValid();
        const bool isRightValid = right.isValid();

        if (isLeftValid == isRightValid)
            return threeWayCompare(left, right);
        return isLeftValid ? -1 : 1;
    }

...and use it to compare "privateness":

case TransferListModel::TR_PRIVATE:
    return customCompare(leftValue, rightValue);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to add the following function:

    int customCompare(const QVariant &left, const QVariant &right)
    {
        const bool isLeftValid = left.isValid();
        const bool isRightValid = right.isValid();

        if (isLeftValid == isRightValid)
            return threeWayCompare(left, right);
        return isLeftValid ? -1 : 1;
    }

Or maybe convert an existing one into a template:

// this
int customCompare(const QDateTime &left, const QDateTime &right)
// into
template <typename T>
int customCompare(const T &left, const T &right)

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

Successfully merging this pull request may close these issues.

None yet

6 participants