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

Show file tree in Add torrent dialog when there is single file inside folder #4805

Closed
wants to merge 1 commit into from
Closed

Show file tree in Add torrent dialog when there is single file inside folder #4805

wants to merge 1 commit into from

Conversation

SukharevAndrey
Copy link

Torrents containing a single file inside folder were considered as containing just a file and didn't let to rename folder. File tree didn't show because it doesn't make any sense for just a single file without any folders. Now it is fixed:
loco_screen
Previously, extra folder Loco_Roco_2_EUR was created in save path and couldn't be renamed in the dialog, only afterwards.

@SukharevAndrey
Copy link
Author

Maybe some other modules need to apply the same fix. If so, point me to them.

@zywo
Copy link
Contributor

zywo commented Feb 16, 2016

Just tested, it works as expected.

Thank you.

@glassez
Copy link
Member

glassez commented Feb 17, 2016

This feature already implemented as part of #4784.

@SukharevAndrey
Copy link
Author

@glassez You are right. But add dialog seems to be buggy at the moment in your implementation. It doesn't show free space on drive and default save path is empty until mode is switched.
loco_bug
As I can see, your PR is a long term goal and will not be merged soon. My fix is just in a few lines, so it can be merged until your PR is done.

@glassez
Copy link
Member

glassez commented Feb 17, 2016

As I can see, your PR is a long term goal and will not be merged soon. My fix is just in a few lines, so it can be merged until your PR is done.

Ok.
Can you repost your bug report above to my PR?

@@ -355,6 +355,13 @@ bool Utils::Fs::sameFileNames(const QString &first, const QString &second)
#endif
}

bool Utils::Fs::isFileInFolder(const QString &file_path)
Copy link
Member

Choose a reason for hiding this comment

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

My fix is just in a few lines

In this case, please spare the world from some more unnecessary lines of code. This function should not be here. Besides, it can be done easier: m_torrentInfo.filePath(0).contains('/') in addnewtorrentdialog.cpp

Copy link
Author

Choose a reason for hiding this comment

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

Is '/' always a separator in torrent file paths? May it be for example '' in Windows or something like that?

Copy link
Member

Choose a reason for hiding this comment

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

All public class methods returns paths with '/' as separator (unless there is implementation bug).

@SukharevAndrey
Copy link
Author

Rebased and included suggestions.

@sledgehammer999
Copy link
Member

@glassez your PR is pretty close to merging. Does it do the same thing as this PR? If yes, I'll close it.

@glassez
Copy link
Member

glassez commented Mar 1, 2016

Does it do the same thing as this PR?

Almost. This PR will become meaningless after my PR be merged.

@SukharevAndrey
Copy link
Author

Closing PR because it is useless now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants