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: Add history file support #24

Merged
merged 3 commits into from
Sep 28, 2020
Merged

Conversation

WPettersson
Copy link
Contributor

@WPettersson WPettersson commented Sep 12, 2020

This functions as best I can work out, but the configuration needs an
absolute location for the history file. The config file does not
interpret ${HOME} or ~, which might need discussion.

This is for issue #23

This functions as best I can work out, but the configuration needs an
absolute location for the history file. The config file does not
interpret ${HOME} or ~, which might need discussion.

For issue cspeterson#23
The history file will now default to ~/.local/state/splatmoji/history as
per XDG proposal for such things. The tilde is expanded to ${HOME} when
reading the config. This expansion will occur for every variable, as
long as the tilde occurs at the start of the variable value. I don't see
this causing any issues, and could be useful if you have either menu or
typing tools in ~/bin/
@iFreilicht
Copy link

I like it! I read the discussion in #23, and welcome this PR generally, but I'm not sure if ~/.local/state/splatmoji is the correct location for this. According to the XDG Base Directory Specification, there's two potential candidates (emphasis mine):

$XDG_DATA_HOME defines the base directory relative to which user specific data files should be stored. If $XDG_DATA_HOME is either not set or empty, a default equal to $HOME/.local/share should be used.
[...]
$XDG_CACHE_HOME defines the base directory relative to which user specific non-essential data files should be stored. If $XDG_CACHE_HOME is either not set or empty, a default equal to $HOME/.cache should be used.

rofi itself uses .cache for its history function as well. In rofi/#769, some people were unhappy with this, as they deem history not "non-essential", and in that thread .local/share was proposed as an alternative. They now have an option to set this. I think .local/share (or $XDG_DATA_HOME if set) is a good candidate, what do you think?

There also was some discussion in rofi/#747 to use rofi's built-in history mechanism for custom scripts, but that has been rejected for rofi -dmenu piped scripts, so having a custom history like this is probably the right way to go.

@WPettersson
Copy link
Contributor Author

@iFreilicht It may be helpful to skim through #23, the associated bug, as that's where I got the ~/.local/state/splatmoji location from. I admit, I don't know the XDG spec well enough to make such a decision, but it's easy to change.

@iFreilicht
Copy link

iFreilicht commented Sep 22, 2020 via email

@cspeterson
Copy link
Owner

cspeterson commented Sep 22, 2020

Okay so first considering only the "where does the recent selection list live" question... 😁

Where my suggestion came from

My suggestion of ${HOME}/.local/state} came from what I had initially read on the Debian wiki, about "proposals" for a state dir which is still lacking in the Base Dir spec:

Proposal: STATE directory

This is a recurring request/complaint (see this or this) on the xdg-freedesktop mailing list to introduce another directory for state information that does not belong in any of the existing categories (see also home-dir.proposal. Examples for this information are:

history files of shells, repls, anything that uses libreadline
logfiles
state of application windows on exit
recently opened files
last time application was run
emacs: bookmarks, ido last directories, backups, auto-save files, auto-save-list 

The above example information is not essential data. However it should still persist on reboots of the system unlike cache data that a user might consider putting in a TMPFS. On the other hand the data is rather volatile and does not make sense to be checked into a VCS. The files are also not the data files that an application works on.

A default folder for a future STATE category might be: $HOME/.local/state

https://wiki.debian.org/XDGBaseDirectorySpecification

I agree with the maillist conversation there that there is a need for something distinct from "cache" and I think the proposed location is reasonable and I know no better or more standard answer.

So if I'm actually the decider (I guess I am!?! 😅) I suppose that's the answer and I see no requirement for it to be configurable unless someone convinces me otherwise

@iFreilicht
Copy link

Huh, that's very interesting, I had no idea. You are indeed the person to decide, this is your project after all :)

And hey, if enough projects start using state and it becomes a pseudo-standard, maybe it will actually be added to the specification at some point.

@cspeterson
Copy link
Owner

@WPettersson And then for the rest of it: thanks so much for doing the work! I'll take a closer look soon and am delighted to have the new feature
└[∵┌]└[ ∵ ]┘[┐∵]┘

@cspeterson cspeterson merged commit 0039751 into cspeterson:master Sep 28, 2020
@WPettersson WPettersson deleted the history branch September 28, 2020 17:40
@cspeterson
Copy link
Owner

Okay I roped in the new code and adjusted/added tests. Thanks!

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.

3 participants