nurupo / ProjectTox-Qt-GUI

A cross-platform front end for ProjectTox Core library, written in C++11 with use of Qt5
GNU General Public License v3.0
342 stars 116 forks source link

Added settings and a page for telling Tox to save/encrypt logs #23

Closed arcrose closed 11 years ago

arcrose commented 11 years ago

Title's self explanatory. I added a page under the settings that allows the user to tell Tox whether to store their chat history, and if they do, whether to encrypt it. Also provided settings data for this information.

arcrose commented 11 years ago

My last build attempt gave me settingsdialog.o: In function SettingsDialog::SettingsDialog(QWidget*)': settingsdialog.cpp:(.text+0x116): undefined reference toLogStorageSettingsPage::LogStorageSettingsPage(QWidget)' collect2: ld returned 1 exit status make: ** [TOX-Qt-GUI] Error 1

I'm not sure how the build is being put together but it looks like my logstoragesettingspage.hpp and .cpp files weren't added to the list.

nurupo commented 11 years ago

Wow, that's a lot of refactoring for me do to for just two checkboxes, plus there is no sense in having logging options before we have logging implemented.

Here is a list of what is wrong:

  1. Don't use the static typedef array in settings, just use two bool variables
  2. Don't store text in executable's resources if you want it to be translated when multiple language support kicks in
  3. The text is too long, didn't read
  4. Don't dump text in a QLabel, make a question-mark icon next to the checkbox and set the text to be icon's tooltip
  5. Your setters and getters in Settings are omitting "set"/"get" in their name
  6. Setters should not return anything
  7. You should set text, check boxes checked and etc in LogStorageSettingsPage::setGui() only.
  8. Always use curly brackets in all if-statements
  9. Use const Settings when you are just setting the GUI
arcrose commented 11 years ago

I had a go at everything you recommended but wasn't sure about some stuff. Here's what I did:

  1. Done.
  2. Moved the filename to a Settings::ABOUTLOGS constant. I'm not sure about how to determine which file to read for the appropriate language, but this should make it easier to change later.
  3. Done. Shortened it significantly.
  4. Tried this but had some trouble getting the icon where I wanted it and setting the tooltip.
  5. Done. Methods now named appropriately.
  6. Done. Made much cleaner.
  7. Done.
  8. Done.
  9. Done.