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

getProperty(AppOptionKeys.APP_DATA_DIR_KEY) is returning default value #2084

Closed
ManfredKarrer opened this issue Dec 6, 2018 · 4 comments · Fixed by #2090
Closed

getProperty(AppOptionKeys.APP_DATA_DIR_KEY) is returning default value #2084

ManfredKarrer opened this issue Dec 6, 2018 · 4 comments · Fixed by #2090
Assignees
Labels
Milestone

Comments

@ManfredKarrer
Copy link
Contributor

ManfredKarrer commented Dec 6, 2018

If you use option --appName=Bisq-1 the result from environment.getProperty(AppOptionKeys.APP_DATA_DIR_KEY);
is Bisq instead of Bisq-1.
If accessed with environment.getAppDataDir(); it is correct.

The default value is applied.

Caused bug at backup (backups Bisq instead of Bisq-1).

@ManfredKarrer ManfredKarrer changed the title Backup is using always Bisq folder not the one defined in appName getProperty(AppOptionKeys.APP_DATA_DIR_KEY) is returning default value Dec 6, 2018
@ManfredKarrer ManfredKarrer added this to the v0.9.1 milestone Dec 6, 2018
@freimair
Copy link
Contributor

freimair commented Dec 7, 2018

That is because --appDataDir is not set and instead, the default is used:

parser.accepts(AppOptionKeys.APP_DATA_DIR_KEY,
"Application data directory")
.withRequiredArg()
.defaultsTo(BisqEnvironment.DEFAULT_APP_DATA_DIR);

same cause as #2048 (comment)

I would suggest reverting #1961 as @cbeams suggested in #2063 for the https://github.com/bisq-network/bisq/milestone/21.

However, as the changes of #1961 are much welcome and do clean up the command line API pretty good, I would suggest planning a refactoring of the option processing for a future release.

@ManfredKarrer
Copy link
Contributor Author

Maybe we can revert only those parts of #1961 where the setting of the defaults have been changed. Seems those are the problematic issues. Would be good to get that asap to get it shipped with the next release mid next week.

@cbeams
Copy link
Contributor

cbeams commented Dec 7, 2018

Reverting the removal of the description method and its replacement with JOptSimple’s defaultsTo In commit 83e1dd3 would be enough. Reverting the entire PR would be overkill, as the custom HelpFormatter introduced is effectively separate and not causing any issues.

@ManfredKarrer
Copy link
Contributor Author

Ah thanks @cbeams - the benefits of granular clean commits ;-)

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

Successfully merging a pull request may close this issue.

3 participants