Skip to content
This repository has been archived by the owner on Jul 27, 2024. It is now read-only.

Update VGriveClient.vala #72

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

RudahXimenes
Copy link
Contributor

Experimental changes I made in check_deleted_files function, sync_files and others

Experimental changes I made in check_deleted_files function, sync_files and others
@bcedu bcedu self-requested a review January 7, 2020 12:19
src/VGriveClient.vala Outdated Show resolved Hide resolved
src/VGriveClient.vala Show resolved Hide resolved
src/VGriveClient.vala Outdated Show resolved Hide resolved
src/VGriveClient.vala Outdated Show resolved Hide resolved
src/VGriveClient.vala Show resolved Hide resolved
EXPERIMENTAL REMOTE CHECKING
Note: could be incremented checking if the remote file is trased or not. If trashed, exist_remote should be false, otherwise should be true
*/
remote_files = this.list_files(-1, lpath, -1);
Copy link
Owner

Choose a reason for hiding this comment

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

The method list_files returns all the files childs of lpath. Why do you use the list method?
It's more efficient to ask only for the file we are checking with get_file_id instead of listing all the files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put this to try understand better why this delete function is deleting files that exists in both remote and local.

I have 3 guesses:

  1. Could be the remote checking that don't retrieve the files properly; or
  2. Could be the local checking that is retrieving the filenames wrong; or
  3. The escape function that is somehow messing up with the file names.

My thought was try to change the remote checking to make sure it's not it.

I know that is not efficient and I don't want to leave like that, but I want to make sure that the bug keep occurring, so we can put the remote check aside and turn back into get_file_id

Makes sense what I'm trying to do?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, it make sense.

Where I have got more problems is in the escape function with filenames that have "special" characters. Could you give me some "problematic" filename that you have detected?

Copy link
Contributor Author

@RudahXimenes RudahXimenes Jan 11, 2020

Choose a reason for hiding this comment

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

Some special caracters were something like:
~
#
accents in general (á ã õ ó)

But I don't think we need to escape this anymore. I made a lot of changes in the checking functions and seems to work fine with caracters.

@bcedu bcedu added the enhancement New feature or request label Jan 7, 2020
Copy link
Contributor Author

@RudahXimenes RudahXimenes left a comment

Choose a reason for hiding this comment

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

I made some changes, trying to make more consistent deletions on Sync.
With these changes, check_deleted_files() became deprecated, but there is new way to check if the files were deleted.

The functions check_remote_files() and check_local_files() now check if file was deleted or need to upload/download, comparing with the library.

I made a new function delete_files(). It just delete a file and update library. But I'm not sure if I did the library update correctly, if you could check if it's ok I'll be happy! :)

Sync methods working consistently.
I reviewed the code and put it to the test. Works fine.

* Download new remote files
* Upload new local files
* Delete files properly
* Update library consistently
@RudahXimenes
Copy link
Contributor Author

I updated and tested the code. Works fine.

@RudahXimenes RudahXimenes requested a review from bcedu January 11, 2020 12:21
@bcedu
Copy link
Owner

bcedu commented Jan 11, 2020

I updated and tested the code. Works fine.

Nice! I will take a look as sonn as I can!

@kloczek
Copy link

kloczek commented Feb 5, 2021

Looks like patch from this PR does not apply cleanly on 1.6.1

+ /usr/bin/cat /home/tkloczko/rpmbuild/SOURCES/vgrive-Update-VGriveClient.vala.patch
+ /usr/bin/patch -p1 -s --fuzz=0 --no-backup-if-mismatch
5 out of 6 hunks FAILED -- saving rejects to file src/VGriveClient.vala.rej
5 out of 5 hunks FAILED -- saving rejects to file src/VGriveClient.vala.rej
5 out of 6 hunks FAILED -- saving rejects to file src/VGriveClient.vala.rej
1 out of 2 hunks FAILED -- saving rejects to file src/VGriveClient.vala.rej
1 out of 2 hunks FAILED -- saving rejects to file src/VGriveClient.vala.rej
3 out of 3 hunks FAILED -- saving rejects to file src/VGriveClient.vala.rej
1 out of 1 hunk FAILED -- saving rejects to file src/VGriveClient.vala.rej
4 out of 4 hunks FAILED -- saving rejects to file src/VGriveClient.vala.rej
6 out of 8 hunks FAILED -- saving rejects to file src/VGriveClient.vala.rej

Are you going to updaye that PR? (or remove current on and rediff against 1.6.1)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants