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

Respect XDG basedir specification on Linux systems #181

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

mgeismann
Copy link

@mgeismann mgeismann commented Jun 27, 2023

Commit message:

_So far, ImSwitch polluted users' home directory on Linux based operating systems. This commit handles ImSwitch related user files in an XDG compatible way: configs go to $XDG_CONFIG_HOME and other (data) files go to $XDG_DATA_HOME.

On other operating systems the 'ImSwitchConfig' folder will be created as before (except that it is now called 'ImSwitch') but will now contain subfolders; one for configs and another one for data.

All occurences of 'UserFileDirs' were replaced by either 'UserConfigFileDirs' or 'UserDataFileDirs' according to the nature of the occurence. One could argue to introduce yet another folder for logs but this case is not clearly handled by the XDG spec so for now they go to the data folder.

The function 'openUserDir()' now opens the config folder; this might need revision. Maybe another function/GUI element should be added to access the data folder._

further comments

  • I also switched to 'pathlib' for handling paths.
  • Similar changes could be implemented for windows (.AppData folder) and Mac.
  • The renaming and change of folder structure should be discussed.
  • There is a new but minor dependency 'xdg' from the PyPi.

I only tested briefly when I made these changes. The branch was rebased on the current 'dev' branch, but were not tested again. So consider this work in progress.

In any case, I would love to hear your feedback!

So far, ImSwitch polluted users' home directory on Linux based operating systems.
This commit handles ImSwitch related user files in an XDG compatible way:
configs go to $XDG_CONFIG_HOME and other (data) files go to $XDG_DATA_HOME.

On other operating systems the 'ImSwitchConfig' folder will be created as before
(except that it is now called 'ImSwitch') but will now contain subfolders; one
for configs and another one for data.

All occurences of 'UserFileDirs' were replaced by either 'UserConfigFileDirs' or
'UserDataFileDirs' according to the nature of the occurence. One could argue to
introduce yet another folder for logs but this case is not clearly handled by
the XDG spec so for now they go to the data folder.

The function 'openUserDir()' now opens the config folder; this might need
revision. Maybe another function/GUI element should be added to access the data
folder.
@jacopoabramo
Copy link
Collaborator

jacopoabramo commented Jun 27, 2023

Hey @mgeismann, thanks for the contribution - awesome that you manage to investigate some stuff about ImSwitch for linux. I mostly use Windows myself so I didn't have the chance to deal with this.

Just to be on the same page, you renamed the configuration folder in this PR from ImSwitchConfig to ImSwitch (the original folder name), right?

I'll try to find some time to review the PR in its entirety.

else:
# on any other OS put data and configs into same folder
if platform.system() == 'Windows': # Windows system, try to return documents directory
# TODO why would these files not go into .AppData\Roaming?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I personally prefer to keep these files into "Documents" because it's easier to access - especially if somebody wants to make a new configuration file for a new setup. At some point we will have to add a configuration manager to be able to build up your own setup from the GUI but right now it's best to keep the config folder in an easier spot to reach.

@mgeismann
Copy link
Author

Hey @mgeismann, thanks for the contribution - awesome that you manage to investigate some stuff about ImSwitch for linux. I mostly use Windows myself so I didn't have the chance to deal with this.

Just to be on the same page, you renamed the configuration folder in this PR from ImSwitchConfig to ImSwitch (the original folder name), right?

I'll try to find some time to review the PR in its entirety.

Yes, I renamed the folders because the XDG spec divides the files between data and config folders and the functions that retrieve the paths are unified this way.

@mgeismann mgeismann marked this pull request as draft June 27, 2023 11:06
@jacopoabramo
Copy link
Collaborator

Hi @mgeismann , is there any update on this?

@mgeismann
Copy link
Author

Hi @mgeismann , is there any update on this?

Hey, frankly: no. This got lost among other things. I have some time right now and will look into it and hopefully give you an update soon!

os.path.expanduser() returns a string. The affected line of code assigns to a
variable that is supposed to be a pathlib.Path and is later used with the '/'
operator (in this case path concatenation) that strings do not support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants