cspeterson / splatmoji

Quickly look up and input emoji and/or emoticons/kaomoji on your GNU/Linux desktop via pop-up menu.
MIT License
212 stars 19 forks source link

History doesn't seem to work #26

Closed farzadmf closed 4 years ago

farzadmf commented 4 years ago

Hi,

First of all, thank you for what you've done. I seem to have problems making history work. This is my config:

history_file=~/.config/splatmoji/history
history_length=10
rofi_command=rofi -dmenu -p splatmoji -i -theme purple -columns 2 -drun-match-fields categories -matching fuzzy -sorting-method levenshtein
xdotool_command=xdotool sleep 0.2 type --delay 100
xsel_command=xsel -b -i

I think the history is supposed to be shown at the top, but I don't see anything.

cspeterson commented 4 years ago

Whoops! Sorry, this is my fault - a bad example provided inside the config file.

Fix is simple: be explicit for the history file path, as the tilde will not be expanded. E.g. history_file=/home/myusername/path/to/thing

Let it be noted that @WPettersson was 100% when he pointed this out as a problem and I just failed to clean it up before it caught you. I'll be pushing a correction shortly! 😇

farzadmf commented 4 years ago

Thank @cspeterson for your reply. I tried to full path, and it doesn't seem to change anything :slightly_frowning_face:

I guess there's no need for a "restart" or something, right? Every time I start splatmoji, it should read the updated config file, right?

I also tried to create the file manually, but the history isn't showing up

WPettersson commented 4 years ago

The config file should be freshly read every time, yes. I'm trying to work out what could be going wrong, but I'm not sure. Depending on how comfortable you are with the idea, you could add echo ${history_file} lines to collect_data_files() in the lib/functions file to begin with, to make sure the config file is read properly.

farzadmf commented 4 years ago

I'm good with the idea 🙂

So, I added this line to there:

  echo history_file = "'${history_file}'"

And this is the output when I do splatmoji type:

history_file = ''
WPettersson commented 4 years ago

That's disconcerting. What if you use splatmoji -v type in a terminal? It should still not store history, but it'll show the config in the terminal.

farzadmf commented 4 years ago

Yup, it shows this:

# Config:
'history_length': '10'
'xdotool_command': 'xdotool sleep 0.2 type --delay 100'
'history_file': '/home/farzad/.config/splatmoji/history'
'xsel_command': 'xsel -b -i'
'rofi_command': 'rofi -dmenu -p splatmoji -i -theme DarkBlue -columns 2 -drun-match-fields categories -matching fuzzy -sorting-method levenshtein'
h
WPettersson commented 4 years ago

Okay ... I don't know why history_file wouldn't work then. Can you try either applying https://gist.github.com/WPettersson/3f1e2384e029aeb8044221779c1b6da0, or adding debug prints to both branches of the if on line 41?

Actually, just to check, are you running the latest git version of splatmoji, or a packaged version from somewhere?

farzadmf commented 4 years ago

I'm gonna try the patch in a bit, but about the package, I'm using the splatmoji-git AUR package, so I'm guessing it should be the latest git version

UPDATE about the patch: when I open /usr/share/splatmoji, my line 41 is very different than what you posted (and the one I see in GitHub), I have something like this for the if statement:

if [ -n "${VERBOSE}" ]; then
  echo 1>&2
  echo '# Data files specified via cli:' 1>&2
  # ...
fi

So I guess the AUR package isn't installing the latest version?

WPettersson commented 4 years ago

That does sound like an old version. If you re-build the AUR package (run makepkg again with appropriate options) it should download the latest git once more.

farzadmf commented 4 years ago

Hey @WPettersson , I re-installed the package, and the history seems to be working fine now 🙂

Just one suggestion: I think it's a good idea to have a history similar to what rofimoji has, where it shows a separate row of history, along with shortcuts alt + 0 ... 9 to enter them

I'm gonna close this since it doesn't seem to be an issue with the latest result

cspeterson commented 4 years ago

@farzadmf first of all I didn't know about rofimoji before this moment so thanks for sharing! 🙂

So I'll make myself more familiar and have a think about changing the UI operations, thanks!

cspeterson commented 4 years ago

PS I have no idea how AUR works nor do I know who created that out-of-date one

I'm totally open to making Pacman packages though, as it appears that it is a supported for fpm (by the packing tool I'm currently using) so lemmee see what I can do 😉

cspeterson commented 4 years ago

and @WPettersson ty for sorting this out! 😅