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

[WIP] XDG Base Dir Spec followup. #383

Merged
merged 5 commits into from Mar 6, 2019

Conversation

@denisfa
Copy link
Collaborator

denisfa commented Mar 6, 2019

@rkitover

As we previously discussed, these were the next steps to support the spec. So keep This includes:

What was done

  1. Fix #ifdef to support more platforms for the spec.

  2. Changing dirs from DOT_DIR .vbam to a XDG Dir
    ${XDG_DATA_HOME:-~/.local/share}/visualboyadvance-m. This is for the SDL port and it is mainly for Unix.

  3. Migration. From vbam.conf to vbam.ini automatically. This includes a appropriate error message from wxWidgets.

Testing

  1. As discussed #94.

  2. Also more tested by me manually. I build the SDL port, run a game and try to save the different types of files and see if theu go into the XDG dir.

  3. Manual testing. There are 4 cases: (1) vbam.ini exists and vbam.conf doesn't; (2) vbam.ini and vbam.conf exist; (3) neither file exist; (4) vbam.conf exists and vbam.ini doesn't. The results were:

(1) No warning message displayed. Configuration loaded as set in vbam.ini;
(2) Warning message displayed from wxWidgets; uses configuration from vbam.ini;
(3) Creates new vbam.ini;
(4) vbam.conf is renamed to vbam.ini and its configuration is loaded.

In case you want to test anything more, clone my repo and branch instead of merging to the main repo.
I had to refactor some coding from previous commits. As usual, keep your eye on code quality! There is always room for improvement!

denisfa added 3 commits Mar 3, 2019
We migrate from 'vbam.conf' to 'vbam.ini' automatically.
@denisfa

This comment has been minimized.

Copy link
Collaborator Author

denisfa commented Mar 6, 2019

The CI/CD check failed due to this:

Error: An exception occurred within a child process:
  DownloadError: Failed to download resource "giflib"
Download failed: https://downloads.sourceforge.net/project/giflib/giflib-5.1.4.tar.bz2
@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Mar 6, 2019

sourceforge download servers go down constantly, this plagues my build system as well

src/Util.cpp Outdated Show resolved Hide resolved
src/Util.cpp Outdated Show resolved Hide resolved
src/common/ConfigManager.cpp Outdated Show resolved Hide resolved
src/common/ConfigManager.cpp Outdated Show resolved Hide resolved
src/sdl/SDL.cpp Outdated Show resolved Hide resolved
@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Mar 6, 2019

I left some comments on 724188a, looks great thanks!

@denisfa

This comment has been minimized.

Copy link
Collaborator Author

denisfa commented Mar 6, 2019

@rkitover Coded the requested changes. My only doubt is about the mkdir function for windows. Since it only takes a single arg, I had to prepare a macro to handle it. Check if it is ok. It is protected by a _WIN32.

#define mkdir(X,Y) (_mkdir(X))
@rkitover

This comment has been minimized.

Copy link
Collaborator

rkitover commented Mar 6, 2019

Alright, looks awesome, merging this now. Thank you!

@rkitover rkitover merged commit 3645388 into visualboyadvance-m:master Mar 6, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@denisfa denisfa deleted the denisfa:xdg-spec-followup branch Mar 7, 2019
rkitover added a commit that referenced this pull request Mar 7, 2019
Followup on #383.

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.