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

fix: Open symlinked folders in fileview #1650

Merged
merged 57 commits into from
Mar 28, 2024
Merged

Conversation

buhtz
Copy link
Member

@buhtz buhtz commented Feb 22, 2024

Symlink folders in fileview now can be opened via double click in all circumstances.

I have to say that I am not sure about that solution or its implications. The related code is to complex (for me) to be sure.
But on my tests it worked.

Close #1476

@buhtz buhtz marked this pull request as draft February 23, 2024 11:28
@buhtz buhtz marked this pull request as ready for review February 23, 2024 11:29
@buhtz
Copy link
Member Author

buhtz commented Mar 4, 2024

I don't feel well merging this without proper review. So I am waiting. But there is no hurry.

@emtiu emtiu requested review from emtiu and aryoda March 4, 2024 11:33
@aryoda
Copy link
Contributor

aryoda commented Mar 4, 2024

I don't feel well merging this without proper review. So I am waiting. But there is no hurry.

Sorry, I did review this (but without actually running the code) and the only thing I have found is the question about a better name for the canOpenPath function (something like eg. IsExistingPathInsideSnapshotFolder - or shorter).

Just propose a better name (or even decide to leave it as it is for now since there is a function doc string):

backintime/common/snapshots.py

Lines 2623 to 2632 in ed282ad

def canOpenPath(self, path):
"""
``True`` if path is a file inside this snapshot
Args:
path (str): path from local filesystem (no snapshot path)
Returns:
bool: ``True`` if file exists
"""

Is there something special I should test (eg. risky code change)?

@emtiu
Copy link
Member

emtiu commented Mar 4, 2024

I won't have a chance for some time, but I'll be happy to test this in a VM.

@buhtz
Copy link
Member Author

buhtz commented Mar 5, 2024

the only thing I have found is the question about a better name for the canOpenPath function (something like eg. IsExistingPathInsideSnapshotFolder - or shorter).

I renamed now and just used your proposal.

Is there something special I should test (eg. risky code change)?

😁 It is primarily an irrational sensation. I lack a thorough understanding of that section of the code, which hinders me from realistically contemplating its implications. It's nothing serious, just a minor challenge.

No meed to merge it soon. It can wait some weeks if Michael want to test it.

@buhtz buhtz merged commit d33f35e into bit-team:dev Mar 28, 2024
1 check passed
@buhtz buhtz deleted the qt6fix/1476symlinkbug branch March 28, 2024 08:52
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.

FileView can not open symlinked folders
3 participants