notsecure / uTox

Lightweight Tox client
utox.org
GNU General Public License v3.0
597 stars 149 forks source link

Add --datapath option #1054

Closed Ansa89 closed 8 years ago

Ansa89 commented 9 years ago

-- @grayhatter edits

tsudoko commented 9 years ago

Option formatting is extremely inconsistent, the theme option uses --opt arg, while your datapath option uses --opt=arg. I think it should conform to the existing way of doing things or support both formats.

Ansa89 commented 9 years ago

@LittleVulpix: My fault to not search for similar open PR. Why notsecure/uTox#1010 isn't already merged?

@tsudoko: according to xlib/main.c:1064 there is a --set= option, this is what made me using =. BTW I will try to change it.

Ansa89 commented 9 years ago

@tsudoko: now it should work also with --datapath example/dir.

LittleVulpix commented 9 years ago

@Ansa89 No idea why it's not merged. But I am curious how these two end up interacting with each other. Sounds like a recipe for disaster : p

Ansa89 commented 9 years ago

I am curious how these two end up interacting with each other

They won't: only one PR will be merged.

SmoothDude commented 9 years ago

Why only one ? They aren't conflicting with each other and I think it is nice idea to set up custom path for utox data.

Ansa89 commented 9 years ago

@SmoothDude: I thought your PR was the ~same as mine, but now I see the difference. Moreover if you say they don't conflict in any way, then the dicision is up to @irungentoo.

GrayHatter commented 9 years ago

They might both be merged, or maybe neither. They don't conflict and both do different things.

Ansa89 commented 9 years ago

@GrayHatter:

GrayHatter commented 9 years ago

@Ansa89 this looks good, if you can change the order in my last comment, or give a reason you want --portable to override --datapath It'll be good to go

Ansa89 commented 9 years ago

@GrayHatter: what should be the correct behavior when both --portable and --datapath options are given at the same time?

GrayHatter commented 9 years ago

@Ansa89 if --datapath is set, and it resolves to a valid directory that exsts and in readable and writable, utox should ignore the --.portable switch. theres' two votes for datapath (--datapath & the datapath itself) and only one for portable....

that or utox should refuse to start becasue they options are conflicting

LittleVulpix commented 9 years ago

I think --portable is basically implicitly 'on' when you specify --datapath, because all you are doing is specifying portable mode - with the actual directory.

Ansa89 commented 9 years ago

@GrayHatter: now --portable is ignored if a valid --datapath is specified. @LittleVulpix: the main difference is that --portable is able to create its directory, while --datapath rely on an already existing path.

GrayHatter commented 9 years ago

If path-given-by-user >450 char, warn about path length and data loss

GrayHatter commented 9 years ago

what happens when the last char is or isn't a / (or a \ on windows)

GrayHatter commented 9 years ago

see OP

Ansa89 commented 9 years ago

If path-given-by-user >450 char, warn about path length and data loss

Why 450 chars? Isn't enough debug("Using \"%s\" as data path\n", user_datapath);?

what happens when the last char is or isn't a / (or a \ on windows)

In both cases an additional "/" or "\" is added when the function int datapath(uint8_t *dest) is called.

GrayHatter commented 9 years ago

I picked 450 out of the air, but UTOX_FILE_NAME_LENGTH is 1024, when I wrote it was was only 512, so I was worried about overruns with long filenames. This shouldn't be a problem here.

GrayHatter commented 9 years ago

@Ansa89 looks good man thank you!

Once @irungentoo takes a look he'll merge it in.