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 Windows filename issue, update Y/n Prompt, update readme #918

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ Optionally set the `GOOGLE_API_CLIENT_ID` and `GOOGLE_API_CLIENT_SECRET` environ

### Hyphens: - vs --

A single hypen `-` can be used to specify options. However two hypens `--` can be used with any options in the provided examples below.
A single hyphen `-` can be used to specify options. However two hyphens `--` can be used with any options in the provided examples below.

### Initializing

Expand Down Expand Up @@ -198,7 +198,7 @@ The opposite of `drive init`, it will remove your credentials locally as well as
drive deinit [-no-prompt]
```

For a complete deinit-ialization, don't forget to revoke account access, [please see revoking account access](#revoking-account-access)
For a complete deinitialization, don't forget to revoke account access, [please see revoking account access](#revoking-account-access)

### Traversal Depth

Expand Down Expand Up @@ -1420,10 +1420,10 @@ Background sync is not just hard, it is stupid. Here are my technical and philos

## Known Issues

* It probably doesn't work on Windows.
Copy link
Owner

Choose a reason for hiding this comment

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

Yeah unfortunately I personally don't have Windows machines nor would I be able to verify that it works properly on Windows so I am hesitant about removing this statement.

Copy link
Author

Choose a reason for hiding this comment

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

I've added back a modified statement: that it's unsupported but works somewhat.

* Google Drive allows a directory to contain files/directories with the same name. Client doesn't handle these cases yet. We don't recommend you to use `drive` if you have such files/directories to avoid data loss.
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather leave this statement in there because drive has to ask folks to fix name clashes(or it can do it automatically). There is also a force mode that just overwrites.

Copy link
Author

Choose a reason for hiding this comment

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

I've added it back with a link to learn more.

* Racing conditions occur if remote is being modified while we're trying to update the file. Google Drive provides resource versioning with ETags, use Etags to avoid racy cases.
* Race conditions occur if remote is being modified while we're trying to update the file. Google Drive provides resource versioning with ETags, use Etags to avoid racy cases.
* drive rejects reading from namedPipes because they could infinitely hang. See [issue #208](https://github.com/odeke-em/drive/issues/208).
* It is unsupported on Windows. Some functionality works but it has not been thoroughly tested.
* Google Drive allows a directory to contain files/directories with the same name. Client doesn't handle these cases gracefully but can offer to 'fix clashes'. See #detecting-and-fixing-clashes . We don't recommend you to use `drive` if you have such files/directories to avoid data loss.

## Reaching Out

Expand Down
4 changes: 4 additions & 0 deletions cmd/drive/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2057,6 +2057,10 @@ func relativePathsOpt(root string, args []string, leastNonExistant bool) ([]stri
relPath = ""
}

// This function appears to be meant to return server-side (/-separated) paths.
// But its input is client side (maybe / or \) separted paths.
// filepath.ToSlash fixes that.
relPath = filepath.ToSlash(relPath)
Copy link
Owner

Choose a reason for hiding this comment

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

Did you mean to instead replace:

- relPath = "/" + relPath
+ relPath = filepath.ToSlash(relPath)

instead of keeping both?

Copy link
Author

@matthewblain matthewblain Sep 30, 2017

Choose a reason for hiding this comment

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

No. It's been a while so I forget exactly what happened (see issue#917) , but it was something like on Windows the input might be something like "photos\myalbum". Which should then be converted into "/photos/myalbum" for use on the remove side, but instead was being turned in to "/photos\myalbum". So both the ToSlash (convert "photos\myalbum" into "photos/myalbum") and the "/" prepend are necessary.

relPath = "/" + relPath
relPaths = append(relPaths, relPath)
}
Expand Down
2 changes: 1 addition & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ func (c *Context) DeInitialize(prompter func(...interface{}) bool, returnOnAnyEr
}

for _, p := range pathsToRemove {
if !prompter("remove: ", p, ". This operation is permanent (Y/N) ") {
if !prompter("remove: ", p, ". This operation is permanent (Y/n) ") {
Copy link
Owner

Choose a reason for hiding this comment

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

How come the change of Y/n? I was thinking it is representative of Yes/No instead of Yes/no.

Copy link
Author

Choose a reason for hiding this comment

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

continue
}

Expand Down
2 changes: 1 addition & 1 deletion src/clashes.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func autoRenameClashes(g *Commands, clashes []*Change) error {
for _, r := range renames {
g.log.Logf("%v %v -> %v\n", r.originalPath, r.change.Src.Id, r.newName)
}
status := promptForChanges("Proceed with the changes [Y/N] ? ")
status := promptForChanges("Proceed with the changes [Y/n] ? ")
if !accepted(status) {
return ErrClashFixingAborted
}
Expand Down
2 changes: 1 addition & 1 deletion src/misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func nextPage() bool {

func promptForChanges(args ...interface{}) Agreement {
argv := []interface{}{
"Proceed with the changes? [Y/n]:",
"Proceed with the changes? [Y/n]: ",
}
if len(args) >= 1 {
argv = args
Expand Down
6 changes: 6 additions & 0 deletions src/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -1096,8 +1096,14 @@ func (r *Remote) findByPathRecvRawM(parentId string, p []string, trashed bool) *
working = false
break
}

if f != nil {
chanOChan <- r.findByPathRecvRawM(f.Id, rest, trashed)
} else {
// Ensure that we properly send
// back nil even if files were not found.
// See https://github.com/odeke-em/drive/issues/933.
chanOChan <- wrapInPaginationPair(nil, nil)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/trash.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func (g *Commands) reduceForTrash(args []string, opt *trashOpt) error {
}

if opt.permanent && g.opts.canPrompt() {
status := promptForChanges("This operation is irreversible. Continue [Y/N] ")
status := promptForChanges("This operation is irreversible. Continue [Y/n] ")
if !accepted(status) {
return status.Error()
}
Expand Down