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

Gtk Port: Quote of the day pull request #1153

Open
wants to merge 11 commits into
base: gtk_next
Choose a base branch
from

Conversation

TheShadowOfHassen
Copy link
Contributor

This is part of my requests in issue #1112 It's the quote of the day. It's in the general tab

I didn't get news fetching because the manuksript blog doesn't have a rss feed. Which is something that would be cool but I know @TheJackiMonster you don't have permissions to do that.

I have three quotes. If people want to make more that's fine as look as they compare with the standard I made. ( It has to be clean and has to pertain to writing.)

Two things which really don't pertain to this pull request but I want to point out.

  1. I want to add a news fetcher and progress banners eventually and I want to put them in the the general tab also. Maybe we should eventually rename the tab to overview to reflect this.
  2. Not a single error pops up if it's in the UI declaring. the UI pane just refuses to load. That makes it really hard when it comes to tracking errors.

@TheShadowOfHassen TheShadowOfHassen changed the title Quote of the day pull request Gtk Port: Quote of the day pull request Apr 8, 2023
@TheShadowOfHassen
Copy link
Contributor Author

I think I've finished the development of this pull request. It'd be great if someone could check the program and the rules out and see what they think!

@TheJackiMonster TheJackiMonster changed the base branch from gtk to gtk_next August 3, 2023 21:48
@TheJackiMonster
Copy link
Collaborator

TheJackiMonster commented Aug 3, 2023

I think I've finished the development of this pull request. It'd be great if someone could check the program and the rules out and see what they think!

I think the idea is solid. The implementation could be adjusted at some points:

  • Currently the quoteOfTheDay.py file contains a lot of code running globally when imported. That's not good for multiple reasons (you don't know when it's executed exactly, how often and what's the context). So I'd recommend putting that inside your functions for reliability. It's also better for maintenance reasons (you don't overlook code outside a function when trying to fix a problem of a function).
  • There's a potential IO issue when reading the file for quotes. You don't check whether open() fails or not. I'd recommend using the with open(path, mode) as var: syntax which gives you a block to use the valid file object. As soon as the block ends, the file will be closed (you don't want to block file access unnecessarily which is another issue in the current implementation) and the block will not be used when opening the file fails. So you can provide defaults to return in that case (for example an empty list).
  • You can reduce some code by using readlines() instead of read() and split(). That should also work cross-platform, I assume (the other might not work exactly as intended in some cases).
  • For consistency reasons I'd like the ids of widgets use _ instead of '-' to concat words. So in the case here quote_label instead of quote-label.

Translation can be addressed at later stages.

@TheShadowOfHassen
Copy link
Contributor Author

OK I'll work on that as soon as I can. I'll be honest I'm not the best programmer and I didn't know about readlines()

@TheShadowOfHassen
Copy link
Contributor Author

I fixed it.

@TheShadowOfHassen
Copy link
Contributor Author

Can this be merged to the gtk next branch?

@TheJackiMonster
Copy link
Collaborator

I think multiple things are still missing. I don't like that the path of the file to load quotes from is hard-coded. That should at least contain the language as a parameter. I might also make sense to encapsulate the whole functionality inside a class. So you can load the quotes initially in a constructor and use the methods of the object to get the current quote of the day.

Another thing I thought about is that a user might be using Manuskript quite long and writes overnight. Then it would be neat if the quote actually changes. Currently it only sets the quote in UI once.

@TheShadowOfHassen
Copy link
Contributor Author

Another thing I thought about is that a user might be using Manuskript quite long and writes overnight. Then it would be neat if the quote actually changes. Currently it only sets the quote in UI once.

I'll do this when I add the progress tracking banner in the window (once we can save progress in manuskript.

It might also make sense to encapsulate the whole functionality inside a class. So you can load the quotes initially in a constructor and use the methods of the object to get the current quote of the day

I might not be as good as a programmer as you, but I don't see how this would make more sense.

I did add the translation thing.

@TheJackiMonster
Copy link
Collaborator

I might not be as good as a programmer as you, but I don't see how this would make more sense.

Because an attribute in a class wouldn't be a global variable which can potentially be changed from anywhere. If we just store one object in the UI where the quotes are actually needed it's much safer to use and more reliable.

It's also much better to address error handling later on. Currently if the load_quotes() function fails because the file might not exist or any other reason, the whole application might crash.

There are really a lot of reasons to avoid global variables if possible.

@TheShadowOfHassen
Copy link
Contributor Author

Ok. I'll work on it. Where should the class initialize?

@TheJackiMonster
Copy link
Collaborator

Ok. I'll work on it. Where should the class initialize?

In the general view, I suppose.

@TheShadowOfHassen
Copy link
Contributor Author

OK thanks!!!

@TheShadowOfHassen
Copy link
Contributor Author

I got that done. Was there anything else?

@TheJackiMonster
Copy link
Collaborator

I got that done. Was there anything else?

Hmm, looks good to me. But is there a reason you made changes to the .gitignore rules?

@TheJackiMonster
Copy link
Collaborator

I've also tested some adjustments. But I'd like to have your opinion. I think it would look better with some smaller size. Also I think if the quote is aligned to the left it looks better when scaling up the window too much. As alternative we could also center or remove the label above stating "Quote of The Day".

Before:
image

After:
image

What do you think? Maybe we could also scale up the quote depending on the window size. I just think currently it's too big for smaller windows but it looks more fitting when I scale it up as on the screenshots to 1440p. ^^'

@TheShadowOfHassen
Copy link
Contributor Author

Maybe we can put adapting scaling on it, however, at the moment It looks worse small on big screens, that distracting on big screens. I think an adapting scaling would work later.

@obw
Copy link
Contributor

obw commented Sep 14, 2023

One little Question, can we install a little PHP Script on https://www.theologeek.ch/manuskript/??

I could write a little Tool too collect quotes! Most of the People don't have the guts or knowledge, to do something like adding Quotes to a file and commit it to git-hub!

Regards

@TheShadowOfHassen
Copy link
Contributor Author

One little Question, can we install a little PHP Script on https://www.theologeek.ch/manuskript/??

I could write a little Tool too collect quotes! Most of the People don't have the guts or knowledge, to do something like adding Quotes to a file and commit it to git-hub!

Regards

It would be a very good idea, but I'm not sure if we can do that, because we really can't do much on the website. @TheJackiMonster would be able to say more.

@obw
Copy link
Contributor

obw commented Sep 14, 2023

@TheShadowOfHassen worst case I can build a Subdomain, I also will need some Time, the Script as such is simple, but Spamming and DoS... all the lovely things, of web development!

The concept is already done in my brain:

Form:
Quote: Sting max 512 Chars (UTF-8)
Source: string max 128 Chars (UTF-8)
Lang-Code (Translated to language name)
Captcha: One which is only visible, when the service provider of the captcha, thinks he should check for Human;)

Storage: Simple, file-based, per Language one directory. Every quote as json in an own file.

Second simple Page, where the quotes are collected to one simple text file, in the correct format, for the quote function. Checkboxes for a yes/no decision to block not acceptable entries. Then download of one zip file, with all raw data and the compiled file. After this clean the Lang-Folder.

Also needed:

  • Blackword list! (Make Money and his different cryptic ways to write it, alone is a real nice filter for spam! I think I will use the default blacklist from DokuWiki )

Nice to have:

  • perhaps, some kind of hash based db, to minimize the dupes.
  • A automated transfer tool to a translation service, so people can check the quotes in languages, they don't speak / read..

All in all four or six hours coding... Without the war zone internet, twenty minutes!

Also, some tool to find dupes, would be nice! At the moment, when we have more than 100 Quotes in one Language, we will need this!

@TheJackiMonster
Copy link
Collaborator

@obw While I like the idea to make it easier for non-technical people to contribute. I don't think we want to allow completely anonymous contributions to the quotes. Because somebody needs to commit them in the end and the person will be responsible for quotes being unproblematic to users.

Also quotes will be language dependent anyway. So I think we could just make them translation files and use weblate for that because otherwise it's nearly impossible to correct or verify them (we need people who speak the language in each case to verify the quotes are correct).

@obw
Copy link
Contributor

obw commented Sep 16, 2023

@TheJackiMonster this is definitely the better way, I had planned a little admin Page to check the entry, but with this no work from my side will be needed. Except, I have some quotes, in English and Deutsch (German)!

Just make everything ready and give us the Link... I will start with some input!

@TheJackiMonster
Copy link
Collaborator

Just make everything ready and give us the Link... I will start with some input!

I'm not sure what you expect from me. I mean I would suggest to use the same Weblate repository as before. I don't think it makes to open a separate since there are already contributors.

I think most thing we need from this MR is the quotes being read from a file in the current translation file format (as the .ts files in the i18n subdirectory use). I'm not sure whether we want to stick with that exact format for GTK in general though. We will definitely not continue using the .qm files for Qt. But I haven't looked into the recommended way for translation files with GTK.

But we can definitely still convert from the .ts files which seem to use some XML based format.

@TheShadowOfHassen
Copy link
Contributor Author

If we want to get a start on this, @obw didn't' you say you know Qt? You could add the feature to Qt manuskript.

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

3 participants