Skip to content

Commit

Permalink
Merge pull request #365 from ucam-department-of-psychiatry/fix_long_c…
Browse files Browse the repository at this point in the history
…pp_comments

Fix long cpp comments
  • Loading branch information
RudolfCardinal authored Aug 28, 2024
2 parents 5bef015 + 1d9ea36 commit 27882b9
Show file tree
Hide file tree
Showing 280 changed files with 2,513 additions and 1,104 deletions.
34 changes: 34 additions & 0 deletions .github/workflows/clang-format.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
# yamllint disable rule:line-length
name: Run clang-format
# yamllint disable-line rule:truthy
on:
push:
paths:
- '**.cpp'
- '**.h'
- .github/scripts/python_setup.sh
- .github/workflows/clang-format.yml
- tablet_qt/tools/clang_format_camcops.py

jobs:
run-clang-format:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: 3.8

- name: Install virtualenv
run: |
set -eux -o pipefail
${GITHUB_WORKSPACE}/.github/scripts/python_setup.sh
- name: Run clang-format
run: |
set -eux -o pipefail
PYTHON=${HOME}/venv/bin/python
which clang-format-15
which clang-format
${PYTHON} ${GITHUB_WORKSPACE}/tablet_qt/tools/clang_format_camcops.py findlongcomments --ignore_urls
2 changes: 1 addition & 1 deletion server/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@
"amqp==5.0.6", # for celery
"Chameleon==3.8.1", # for Deform
"tornado==6.4.1", # for celery
"webob==1.8.6", # for pyramid
"webob==1.8.8", # for pyramid
]


Expand Down
19 changes: 12 additions & 7 deletions tablet_qt/common/colourdefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,12 @@ More detail on (4)/(5), with working thoughts
const QColor TEST_COLOUR("purple");
-> qcolor.h
inline QColor(const char *aname) : QColor(QLatin1String(aname)) {}
inline QColor(const char *aname) : QColor(QLatin1String(aname))
{}
-> qcolor.h
inline QColor::QColor(QLatin1String name) { setNamedColor(aname); }
inline QColor::QColor(QLatin1String name) {
setNamedColor(aname);
}
-> qcolor.cpp
void QColor::setNamedColor(const QString &name) {
setColorFromString(qToStringViewIgnoringNull(name));
Expand All @@ -104,7 +107,8 @@ More detail on (4)/(5), with working thoughts
-> qcolor.cpp
static bool get_named_rgb(const char *name, int len, QRgb* rgb)
-> qcolor.cpp
static bool get_named_rgb_no_space(const char *name_no_space, QRgb *rgb)
static bool get_named_rgb_no_space(const char *name_no_space,
QRgb *rgb)
// reads rgbTbl
qcolor.cpp:
Expand All @@ -120,10 +124,10 @@ More detail on (4)/(5), with working thoughts
can be called, since they're in the same compilation unit. [INCORRECT]
Possibly this is a Visual C++ bug; see
https://blogs.msdn.microsoft.com/vcblog/2016/03/31/visual-c-2015-update-2-bug-fixes/
https://blogs.msdn.microsoft.com/vcblog/2016/03/31/visual-c-2015-update-2-bug-fixes/
and search for "init".
I'm using (2018-04-29) Visual C++ 19.00.23918, which is Update 2 of VS 2015.
Let's try Visual Studio Community 2017.
I'm using (2018-04-29) Visual C++ 19.00.23918, which is Update 2 of VS
2015. Let's try Visual Studio Community 2017.
- install the "C++" (and "C++ for Linux") options
- goes into C:\Program Files (x86)\Microsoft Visual Studio\2017
Expand All @@ -141,7 +145,8 @@ More detail on (4)/(5), with working thoughts
You can also run
C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvarsall.bat amd64
C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\
Auxiliary\Build\vcvarsall.bat amd64
or similar, and then when you run "cl.exe" it'll pick the right one.
This gives Visual Studio 2017 Developer Command Prompt of 15.6.7 and a
Expand Down
24 changes: 16 additions & 8 deletions tablet_qt/common/gui_defines.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,22 @@ Files that make up a complete set of classes:
Relevant web discussions include:
Qt docs: https://doc.qt.io/qt-6.5/layout.html#layout-issues
2009-01-16: http://stackoverflow.com/questions/452333/how-to-maintain-widgets-aspect-ratio-in-qt
2011-11-21: http://stackoverflow.com/questions/8211982/qt-resizing-a-qlabel-containing-a-qpixmap-while-keeping-its-aspect-ratio
2012-12-31: http://stackoverflow.com/questions/14104871/qlabel-cutting-off-text-on-resize
2013-01-09: http://stackoverflow.com/questions/14238138/heightforwidth-label
2014-06-17: http://stackoverflow.com/questions/24264320/qt-layouts-keep-widget-aspect-ratio-while-resizing
2015-03-30: http://www.qtcentre.org/threads/62059-QLabel-Word-Wrapping-adds-unnecessary-line-breaks
2015-07-21: http://stackoverflow.com/questions/31535143/how-to-prevent-qlabel-from-unnecessary-word-wrapping
Qt docs:
https://doc.qt.io/qt-6.5/layout.html#layout-issues
2009-01-16:
http://stackoverflow.com/questions/452333/how-to-maintain-widgets-aspect-ratio-in-qt
2011-11-21:
http://stackoverflow.com/questions/8211982/qt-resizing-a-qlabel-containing-a-qpixmap-while-keeping-its-aspect-ratio
2012-12-31:
http://stackoverflow.com/questions/14104871/qlabel-cutting-off-text-on-resize
2013-01-09:
http://stackoverflow.com/questions/14238138/heightforwidth-label
2014-06-17:
http://stackoverflow.com/questions/24264320/qt-layouts-keep-widget-aspect-ratio-while-resizing
2015-03-30:
http://www.qtcentre.org/threads/62059-QLabel-Word-Wrapping-adds-unnecessary-line-breaks
2015-07-21:
http://stackoverflow.com/questions/31535143/how-to-prevent-qlabel-from-unnecessary-word-wrapping
*/

Expand Down
3 changes: 2 additions & 1 deletion tablet_qt/common/preprocessor_aid.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
// GCC defines __GNUC__.
// Visual C++ defines _MSC_VER.

#if defined __clang__ // NB defined in Qt Creator; put this first for that reason
#if defined __clang__
// NB defined in Qt Creator; put this first for that reason
#define COMPILER_IS_CLANG
#if __clang_major__ >= 10
#define CLANG_AT_LEAST_10
Expand Down
3 changes: 2 additions & 1 deletion tablet_qt/common/textconst.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ class TextConst : public QObject
static QString defaultHintText();
static QString DEFUNCT_SYMBOL;
static QString defunctSubtitleSuffix();
static QString delete_(); // "delete" is a C++ keyword; "DELETE" also breaks Visual C++
static QString delete_();
// ... "delete" is a C++ keyword; "DELETE" also breaks Visual C++
static QString description();
static QString diagnosis();

Expand Down
12 changes: 8 additions & 4 deletions tablet_qt/common/uiconst.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ const QFont::Weight QCALENDARWIDGET_HEADER_FONTWEIGHT = QFont::Bold;
const QColor QCALENDARWIDGET_TEXT_WEEKDAY(0, 0, 0); // black
const QColor QCALENDARWIDGET_TEXT_WEEKEND(255, 0, 0); // red
const QDate QCALENDARWIDGET_MIN_DATE(1880, 1, 1); // no older
const QDate QCALENDARWIDGET_MAX_DATE = QDate(); // QCalendarWidget default ("no limit")
const QDate QCALENDARWIDGET_MAX_DATE = QDate();
// ... QCalendarWidget default ("no limit")

const QMargins NO_MARGINS(0, 0, 0, 0);

Expand All @@ -96,9 +97,12 @@ const QString CSS_CAMCOPS_DIAGNOSTIC_CODE(

const QString WARNING_COLOUR_CSS("red");

const QColor BUTTON_UNPRESSED_COLOUR(127, 127, 127, 100); // translucent mid-grey
const QColor BUTTON_PRESSED_COLOUR(100, 100, 255, 200); // translucent light blue
const QColor BUTTON_DISABLED_COLOUR(127, 127, 127, 200); // very translucent mid-grey
const QColor BUTTON_UNPRESSED_COLOUR(127, 127, 127, 100);
// ... translucent mid-grey
const QColor BUTTON_PRESSED_COLOUR(100, 100, 255, 200);
// ... translucent light blue
const QColor BUTTON_DISABLED_COLOUR(127, 127, 127, 200);
// ... very translucent mid-grey
const qreal DISABLED_ICON_OPACITY = 0.5;

// ============================================================================
Expand Down
3 changes: 2 additions & 1 deletion tablet_qt/common/uiconst.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ extern const int DEFAULT_COLSPAN_Q;
extern const int DEFAULT_COLSPAN_A;
extern const int MIN_SPINBOX_HEIGHT_FOR_DEFAULT_DPI;
extern int g_min_spinbox_height; // not const!
extern const QAbstractSpinBox::ButtonSymbols SPINBOX_SYMBOLS; // how to display a spinbox
extern const QAbstractSpinBox::ButtonSymbols SPINBOX_SYMBOLS;
// ... how to display a spinbox
extern const int SLIDER_HANDLE_SIZE_PX_FOR_DEFAULT_DPI;
extern int g_slider_handle_size_px; // not const!
extern const int SLIDER_GROOVE_MARGIN_PX;
Expand Down
77 changes: 49 additions & 28 deletions tablet_qt/core/camcopsapp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,12 @@
#include "db/sqlcipherdriver.h"
#endif

const QString APPSTRING_TASKNAME("camcops"); // task name used for generic but downloaded tablet strings
const QString APP_NAME("camcops"); // e.g. subdirectory of ~/.local/share; DO NOT ALTER
const QString APP_PRETTY_NAME("CamCOPS"); // main window title and suffix on dialog window titles
const QString APPSTRING_TASKNAME("camcops");
// ... task name used for generic but downloaded tablet strings
const QString APP_NAME("camcops");
// ... e.g. subdirectory of ~/.local/share; DO NOT ALTER
const QString APP_PRETTY_NAME("CamCOPS");
// ... main window title and suffix on dialog window titles
const QString CONNECTION_DATA("data");
const QString CONNECTION_SYS("sys");
const int DEFAULT_SERVER_PORT = 443; // HTTPS
Expand All @@ -120,11 +123,13 @@ const int UPLOAD_INTERVAL_SECONDS = 10 * 60; // 10 minutes
CamcopsApp::CamcopsApp(int& argc, char* argv[]) :
QApplication(argc, argv),
m_p_task_factory(nullptr),
m_lockstate(LockState::Locked), // default unless we get in via encryption password
m_lockstate(LockState::Locked),
// ... default unless we get in via encryption password
m_p_main_window(nullptr),
m_p_window_stack(nullptr),
m_p_hidden_stack(nullptr),
m_maximized_before_fullscreen(true), // true because openMainWindow() goes maximized
m_maximized_before_fullscreen(true),
// ... true because openMainWindow() goes maximized
m_patient(nullptr),
m_storedvars_available(false),
m_netmgr(nullptr),
Expand Down Expand Up @@ -384,7 +389,8 @@ bool CamcopsApp::registerPatientWithServer()
new_patient_proquint = m_default_patient_proquint;
} else {
// Start with a blank URL, or a URL from a previous failed attempt, to
// assist in reducing data entry following network/registration failure.
// assist in reducing data entry following network/registration
// failure.
QUrl old_server_url = QUrl();
const QString old_patient_proquint = varString(varconst::SINGLE_PATIENT_PROQUINT);
if (!old_patient_proquint.isEmpty()) {
Expand Down Expand Up @@ -713,7 +719,8 @@ void CamcopsApp::maybeRetryNetworkOperation(const QString base_message,
TaskSchedulePtrList CamcopsApp::getTaskSchedules()
{
TaskSchedulePtrList task_schedules;
TaskSchedule specimen(*this, *m_sysdb, dbconst::NONEXISTENT_PK); // this is why function can't be const
TaskSchedule specimen(*this, *m_sysdb, dbconst::NONEXISTENT_PK);
// ... this is why function can't be const
const WhereConditions where; // but we don't specify any
const SqlArgs sqlargs = specimen.fetchQuerySql(where);
const QueryResult result = m_sysdb->query(sqlargs);
Expand Down Expand Up @@ -879,8 +886,10 @@ int CamcopsApp::run()
TextConst::pleaseWait());
}

openMainWindow(); // uses HelpMenu etc. and so must be AFTER TASK REGISTRATION
makeNetManager(); // needs to be after main window created, and on GUI thread
openMainWindow();
// ... uses HelpMenu etc. and so must be AFTER TASK REGISTRATION
makeNetManager();
// ... needs to be after main window created, and on GUI thread

if (varInt(varconst::MODE) == varconst::MODE_NOT_SET) {
// e.g. fresh database; which mode to use?
Expand Down Expand Up @@ -942,7 +951,8 @@ void CamcopsApp::backgroundStartup()
const Version old_version = upgradeDatabaseBeforeTablesMade();
makeOtherTables();
registerTasks(); // AFTER storedvar creation, so tasks can read them
upgradeDatabaseAfterTasksRegistered(old_version); // AFTER tasks registered
upgradeDatabaseAfterTasksRegistered(old_version);
// ... AFTER tasks registered
makeTaskTables();
// Should we drop tables we're unaware of? Clearly we should never do this
// on the server. Doing so on the client prevents the client trying to
Expand Down Expand Up @@ -1767,8 +1777,10 @@ void CamcopsApp::initGuiOne()
m_qt_physical_dpi = uiconst::DEFAULT_DPI;
} else {
const QScreen* screen = all_screens.at(0);
m_qt_logical_dpi.x = screen->logicalDotsPerInchX(); // can be e.g. 96.0126
m_qt_logical_dpi.y = screen->logicalDotsPerInchY(); // can be e.g. 96.0126
m_qt_logical_dpi.x = screen->logicalDotsPerInchX();
// ... can be e.g. 96.0126
m_qt_logical_dpi.y = screen->logicalDotsPerInchY();
// ... can be e.g. 96.0126
// https://stackoverflow.com/questions/16561879/what-is-the-difference-between-logicaldpix-and-physicaldpix-in-qt
m_qt_physical_dpi.x = screen->physicalDotsPerInchX();
m_qt_physical_dpi.y = screen->physicalDotsPerInchY();
Expand Down Expand Up @@ -2040,7 +2052,8 @@ void CamcopsApp::openSubWindow(OpenableWidget* widget, TaskPtr task,
while (m_p_window_stack->count() > 0) {
QWidget* w = m_p_window_stack->widget(m_p_window_stack->count() - 1);
if (w) {
m_p_window_stack->removeWidget(w); // m_p_window_stack still owns w
m_p_window_stack->removeWidget(w);
// ... m_p_window_stack still owns w
m_p_hidden_stack->addWidget(w); // m_p_hidden_stack now owns w
}
}
Expand Down Expand Up @@ -2142,13 +2155,13 @@ void CamcopsApp::closeSubWindow()
// AND 5.9
// - From https://doc.qt.io/qt-6.5/qstackedwidget.html#removeWidget :
// Removes widget from the QStackedWidget. i.e., widget is not deleted
// but simply removed from the stacked layout, causing it to be hidden.
// Note: Ownership of widget reverts to the application.
// but simply removed from the stacked layout, causing it to be
// hidden. Note: Ownership of widget reverts to the application.
// - From https://doc.qt.io/qt-6.5/qstackedwidget.html#removeWidget :
// Removes widget from the QStackedWidget. i.e., widget is not deleted
// but simply removed from the stacked layout, causing it to be hidden.
// Note: Parent object and parent widget of widget will remain the
// QStackedWidget. If the application wants to reuse the removed
// but simply removed from the stacked layout, causing it to be
// hidden. Note: Parent object and parent widget of widget will remain
// the QStackedWidget. If the application wants to reuse the removed
// widget, then it is recommended to re-parent it.
// ... same for Qt 5.11.
// - Also:
Expand All @@ -2159,10 +2172,12 @@ void CamcopsApp::closeSubWindow()
// ------------------------------------------------------------------------
// Restore the widget from the top of the hidden stack
// ------------------------------------------------------------------------
Q_ASSERT(m_p_hidden_stack->count() > 0); // the m_info_stack.isEmpty() check should exclude this
Q_ASSERT(m_p_hidden_stack->count() > 0);
// ... the m_info_stack.isEmpty() check should exclude this
QWidget* w = m_p_hidden_stack->widget(m_p_hidden_stack->count() - 1);
m_p_hidden_stack->removeWidget(w); // m_p_hidden_stack still owns w
const int index = m_p_window_stack->addWidget(w); // m_p_window_stack now owns w
const int index = m_p_window_stack->addWidget(w);
// ... m_p_window_stack now owns w
m_p_window_stack->setCurrentIndex(index);

// ------------------------------------------------------------------------
Expand Down Expand Up @@ -2247,8 +2262,10 @@ void CamcopsApp::enterFullscreen()
// QWidget::showFullScreen does this:
//
// ensurePolished();
// setWindowState((windowState() & ~(Qt::WindowMinimized | Qt::WindowMaximized))
// | Qt::WindowFullScreen);
// setWindowState(
// (windowState() & ~(Qt::WindowMinimized | Qt::WindowMaximized))
// | Qt::WindowFullScreen
// );
// setVisible(true);
// activateWindow();

Expand Down Expand Up @@ -2317,8 +2334,8 @@ void CamcopsApp::leaveFullscreen()
} else {
// Under Linux, the method above doesn't; that takes it to normal mode.
// Under Linux, showMaximized() also takes it to normal mode!
// But under Linux, calling showNormal() then showMaximized() immediately
// does work.
// But under Linux, calling showNormal() then showMaximized()
// immediately does work.
if (m_maximized_before_fullscreen) {
#ifdef DEBUG_SCREEN_STACK
qDebug() << Q_FUNC_INFO << "calling showMaximized() then showMaximized()";
Expand Down Expand Up @@ -2529,7 +2546,8 @@ void CamcopsApp::resetEncryptionKeyIfRequired()
}
qInfo() << "Resetting internal encryption key (and wiping stored password)";
setVar(varconst::OBSCURING_KEY, cryptofunc::generateObscuringKeyBase64());
setVar(varconst::OBSCURING_IV, ""); // will be set by setEncryptedServerPassword
setVar(varconst::OBSCURING_IV, "");
// ... will be set by setEncryptedServerPassword
setVar(varconst::SERVER_USERPASSWORD_OBSCURED, "");
}

Expand Down Expand Up @@ -2714,7 +2732,8 @@ PatientPtrList CamcopsApp::getAllPatients(const bool sorted)

QueryResult CamcopsApp::queryAllPatients()
{
Patient specimen(*this, *m_datadb, dbconst::NONEXISTENT_PK); // this is why function can't be const
Patient specimen(*this, *m_datadb, dbconst::NONEXISTENT_PK);
// ... this is why function can't be const
const WhereConditions where; // but we don't specify any
const SqlArgs sqlargs = specimen.fetchQuerySql(where);

Expand Down Expand Up @@ -3377,8 +3396,10 @@ NetworkManager::UploadMethod CamcopsApp::getUploadMethodFromUser() const
m_p_main_window);
QAbstractButton* copy = msgbox.addButton(TextConst::copy(), QMessageBox::YesRole);
QAbstractButton* move_keep = msgbox.addButton(tr("Keep patients and move"), QMessageBox::NoRole);
QAbstractButton* move = msgbox.addButton(tr("Move"), QMessageBox::AcceptRole); // e.g. OK
msgbox.addButton(TextConst::cancel(), QMessageBox::RejectRole); // e.g. Cancel
QAbstractButton* move = msgbox.addButton(tr("Move"), QMessageBox::AcceptRole);
// ... e.g. OK
msgbox.addButton(TextConst::cancel(), QMessageBox::RejectRole);
// ... e.g. Cancel
msgbox.exec();
QAbstractButton* reply = msgbox.clickedButton();
if (reply == copy) {
Expand Down
6 changes: 4 additions & 2 deletions tablet_qt/core/camcopsapp.h
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,8 @@ protected slots:

// Translators; see https://doc.qt.io/qt-6.5/internationalization.html
QSharedPointer<QTranslator> m_qt_translator; // translates Qt strings
QSharedPointer<QTranslator> m_app_translator; // translates CamCOPS strings
QSharedPointer<QTranslator> m_app_translator;
// ... translates CamCOPS strings

// Database directory.
QString m_database_path;
Expand All @@ -854,7 +855,8 @@ protected slots:
QPointer<QStackedWidget> m_p_window_stack;

// The stack of hidden windows.
QSharedPointer<QStackedWidget> m_p_hidden_stack; // we own it entirely, so QSharedPointer
QSharedPointer<QStackedWidget> m_p_hidden_stack;
// ... we own it entirely, so QSharedPointer

// Before we went fullscreen, were we maximized?
bool m_maximized_before_fullscreen;
Expand Down
Loading

0 comments on commit 27882b9

Please sign in to comment.