Darazaki
Adding the ability to select multiple words at a time
One key part of speed reading is to read more than one word at a time, as to get ideas and not just words, and reduce subvocalizations.
I thought it would be a nice idea to be able to read more than one word at a time on spedread, and I added it.
I also updated the translation files with the positions, and I added the new string in french since I know that language.
Thanks for the PR and sorry for the delay!
I like the idea a lot actually but right now this implementation has a lot of trouble dealing with words separated by a lot of whitespace. This mean we would probably need two different implementation: one for the purple highlighting and one for actually reading the words. Basically, a string like hello \n world should, imo, be highlighted as such but should appear while reading as hello world. I didn't like the words shifting around vertically while reading using your fork but, other than that, it's a really good idea imo!
I don't know if you want to go through with implementing all of that but I'll still do a code review just in case :)
Okay! I think I've finished nitpicking :)
As I said, you don't need to implement all of that if you don't feel like it. I'm not your boss or anything. But I prefer not merging those changes before what the end user see feels polished from my perspective
Alright, I removed the newlines from being displayed in the read part by adding a filter function. Let me know if it works. I also plan on making a second PR that improves on the concept of words at a time. Instead of always showing as an example, 4 words, it should be maximum 4 but it still stops if there is an end of sentence, or a comma, because otherwise when you read it, it feels weird because it's not part of the same context. This PR is built on top of these changes, so I will submit it once this is merged.
What I meant is that whatever whitespace gets in between what would have been considered 2 words by the original SpedreadWindow.next_word implementation should be converted into a space. Sorry if I wasn't clear enough before
E.g.: abc !\n\tdef -> abc ! def
Your current behavior seems to stop at newlines. The thing is, file formats who don't have good representation of newlines (like PDF) usually have reader applications which like adding newlines in the middle of paragraphs. Since I assume that users will copy & paste texts as they are into Spedread, it's better to treat newlines as spaces imo
Also, you accidentally removed the appdata translations in "fix appdata that were not there initially". You should probably git revert e037d9c since it'll be a pain to fix manually
Other than that, I've only skimmed over your code so far but it works very well! Good job :)
I've added the requested change by using regex matching that convert a combination of newlines and whitespaces into a space. If there is anything else, please let me know.
I think I finally understand why you wanted to remove appdata from the translations, translations should be updated through dev-scripts/update-translations.sh rather than manually (I assume that's what caused all the changelog descriptions that were added?). The extra stuff should be removed
At this point you should probably git restore po/ --source=d8210e7 to get the po/ files back to their original state and rerun dev-scripts/update-translations.sh to get your translation string back into the files and then redo the french translation
Then there's the thing where you made _words_at_a_time static, which will lead to all kinds of wrong behaviors. Right now it only seems to work because accessing the last instance of _words_at_a_time that is created each time you make a new window will often give you the correct value. However, if you try opening a 2nd window and then close it to go back to the 1st one to update the value of _words_at_a_time you will start seeing some bugs where what you see isn't what you get
The last thing I wanted you to fix was the indentation and some spaces that were missing (things like my_function(1,2) instead of my_function (1, 2) I noticed in some places)
The other thing I thought about is a nitpick so you don't have to "fix" it. It's just that regexes are compiled on creation so rather than recompiling the regex every time you call filter_new_lines, it'd be better to compile it once on app launch and then keep using it. Making it a static private property of SpedreadWindow should do the trick!
If you have any questions, feel free to ask! I know it's a lot so dw :)
The reason I made words_at_a_time static is because next_word function is static, and it needs this attributes, however next_word also compiles and seems to work if I make it non static as well. Is it okay if it is non static ?
using git restore and then re running dev-scripts/update-translations.sh brings all the appdata stuff with all the long comments like this. #: data/com.github.Darazaki.Spedread.appdata.xml.in:63 #: data/com.github.Darazaki.Spedread.appdata.xml.in:140 #: data/com.github.Darazaki.Spedread.appdata.xml.in:148 #: data/com.github.Darazaki.Spedread.appdata.xml.in:177 #: data/com.github.Darazaki.Spedread.appdata.xml.in:210 #: data/com.github.Darazaki.Spedread.appdata.xml.in:223 msgid "Fixed:" msgstr ""
They are added in the line 7 of your script.
Do you want to keep them or not. Originally, what I did was to remove the line 7 cause it did not seem like we needed to include those
The reason I made words_at_a_time static is because next_word function is static, and it needs this attributes, however next_word also compiles and seems to work if I make it non static as well. Is it okay if it is non static ?
Yes of course! That's even what I suggested during the review :)
Do you want to keep them or not. Originally, what I did was to remove the line 7 cause it did not seem like we needed to include those
No, that's just extremely weird. When I run dev-scripts/update-translations.sh in your fork all the appdata changelog entries get removed. That should be the expected behavior since all changelog descriptions are marked with translatable="no". You can even see it in GNU gettext's documentation: https://www.gnu.org/software/gettext/manual/html_node/ITS-Rules.html
The appdata file has to be included in the generation since it also defines the description of the app which needs to be translated in all languages
Just to check, are you also seeing this output?
Regenerated po/POTFILES.
po/_base.pot: ................ done.
po/cs.po: ............... done.
po/de.po: ................ done.
po/fr.po: ................ done.
po/it.po: ................ done.
po/nl.po: ............... done.
po/oc.po: ............... done.
po/pt_BR.po: ................ done.
po/ru.po: ................ done.
po/tr.po: ................ done.
I suspect we have a different version of gettext which is causing issues. In that case, just restore po/ back and don't worry about adding translations. I'll handle that part myself
What's your xgettext --version? I'd be nice if I can figure out the root cause of this and create a workaround. Here's mine for reference:
xgettext (GNU gettext-tools) 0.26
Copyright (C) 1995-2025 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Written by Ulrich Drepper.
When I run your commmand, I get Regenerated po/POTFILES. po/_base.pot: data/com.github.Darazaki.Spedread.appdata.xml.in:150: AVERTISSEMENT : La chaîne « msgid » qui suit contient des caractères non ASCII. Cela causera un problème aux traducteurs qui utilisent un encodage de caractères différent du vôtre. Vous devriez plutôt créer des chaînes « msgid » en pur ASCII. « Typo in German translation has been fixed (thanks to Jürgen Benvenuti) » data/com.github.Darazaki.Spedread.appdata.xml.in:150: Séquence d'octets multiples invalide data/com.github.Darazaki.Spedread.appdata.xml.in:150: Séquence d'octets multiples invalide data/com.github.Darazaki.Spedread.appdata.xml.in:150: Séquence d'octets multiples invalide data/com.github.Darazaki.Spedread.appdata.xml.in:150: Séquence d'octets multiples invalide data/com.github.Darazaki.Spedread.appdata.xml.in:158: AVERTISSEMENT : La chaîne « msgid » qui suit contient des caractères non ASCII. Cela causera un problème aux traducteurs qui utilisent un encodage de caractères différent du vôtre. Vous devriez plutôt créer des chaînes « msgid » en pur ASCII. « Add German translation (thanks to Jürgen Benvenuti) » data/com.github.Darazaki.Spedread.appdata.xml.in:158: Séquence d'octets multiples invalide data/com.github.Darazaki.Spedread.appdata.xml.in:158: Séquence d'octets multiples invalide data/com.github.Darazaki.Spedread.appdata.xml.in:158: Séquence d'octets multiples invalide data/com.github.Darazaki.Spedread.appdata.xml.in:158: Séquence d'octets multiples invalide ...................... terminé. po/cs.po: ....................... terminé. po/de.po: ............................ terminé. po/fr.po: ........................... terminé. po/it.po: .......................... terminé. po/nl.po: ........................... terminé. po/oc.po: ........................... terminé. po/pt_BR.po: .......................... terminé. po/ru.po: ............................ terminé. po/tr.po: .................... terminé. It keeps saying invalid multiple byte sequences and it wants me to use ASCII and i use version 0.25.1 . It is the last available in fedora
It keeps saying invalid multiple byte sequences and it wants me to use ASCII
That's... even weirder? ASCII actually seems to be the default according to man xgettext so I'm really confused how this even works on my computer now. If you edit dev-scripts/update-translations.sh by adding --from-code=UTF-8 to the xgettext call like so:
xgettext \
--from-code=UTF-8 \
-f po/POTFILES \
-x po/_excluded.pot \
-cTR: \
--omit-header
Does it finally fix the issue with appdata?
No, but it seems to be with a specific caracter, the u with the dots,
If i change it to a u <li>Typo in German translation has been fixed (thanks to Jurgen Benvenuti)</li>
I no longer get the errors.
But it doesn't fix the main problem, which is that translatable=no gets ignored
As a workaround, what I found to be effective was to add the non translatable strings to the _excluded.po file. Now it updated the correct translations.
As a workaround, what I found to be effective was to add the non translatable strings to the _excluded.po file. Now it updated the correct translations.
That sucks, I'll try looking for a nicer solution once this is merged, worst-case scenario I'll create my own tooling for translating appdata. It's a shame that this is the best solution for now. Sorry you had to deal with this whole mess :/
Unless you have anything else to add to this PR, I'll merge it tomorrow (will most likely squash it since there are a lot of commits). Thanks a lot for your contribution! :)
Unless you have anything else to add to this PR, I'll merge it tomorrow (will most likely squash it since there are a lot of commits). Thanks a lot for your contribution! :)
I have nothing else for this PR
I have nothing else for this PR
Perfect thanks! I'll publish all of that in v2.6.0 of Spedread :)
I have a few extra things I'd like to do before releasing v2.6.0 (mostly fixing these pesky translation generation issues) so I'll handle its release sometime this weekend. Thanks again for your patience and contribution and sorry again for the mess this was ^^'