Make more instance variables private.#929
Make more instance variables private.#929ahmedcharles merged 4 commits intoMudlet:developmentfrom ahmedcharles:private
Conversation
SlySven
left a comment
There was a problem hiding this comment.
As well as some comments being miss-placed; won't this reordering of the members in the class b****rmess-up the initialisation list in the Constructor?
src/EAction.h
Outdated
| signals: | ||
| void triggered(QAction*); | ||
|
|
||
| public://private: |
There was a problem hiding this comment.
How does this make them private?
There was a problem hiding this comment.
This doesn't. Making them private now requires more changes. This is effectively a TODO.
There was a problem hiding this comment.
Open the comment with TODO: that is one of the default terms that the Qt Creator "Todo-list" plug-in tracks and indexes and can jump you with a click:
| // What the user has set as their preference | ||
| bool mIsCurrentLogFileInHtmlFormat; | ||
|
|
||
| // What the current file will use, set from the previous |
There was a problem hiding this comment.
And this one (before lines 218 to 224 inclusive) refers to: mIsCurrentLogFileInHtmlFormat.
src/Host.h
Outdated
| bool mPrintCommand; | ||
| QString mPrompt; | ||
|
|
||
| // The following was incorrectly called mRawStreamDump |
There was a problem hiding this comment.
This comment really refers to BOTH of the next two members - and explains why and how the original member's functions was spilt.
src/Host.h
Outdated
| @@ -215,24 +188,9 @@ class Host : public QObject | |||
| // What the user has set as their preference | |||
There was a problem hiding this comment.
This refers to the first one: mIsNextLogFileInHtmlFormat.
src/Host.h
Outdated
| int mPort; | ||
| QString mPrompt; | ||
|
|
||
| // What the current file will use, set from the previous |
There was a problem hiding this comment.
Out of place - see prior comment
|
Just curious, what's with the b****r? I'm not sure I get it. |
|
I removed the commentary on phpBB, etc. That's a completely new feature, so perhaps we should open an issue or forum thread. I'd prefer comments that are TODO/FIXME to be things that are 'wrong' with the current implementation rather than just random ideas about how things could be some day. |
| // To cover the corner case of the user changing the mode | ||
| // whilst a log is being written, this stores the mode of | ||
| // the next log file. See mIsCurrentLogFileInHtmlFormat. | ||
| bool mIsNextLogFileInHtmlFormat; |
There was a problem hiding this comment.
Suggested revised text:
// To cover the corner case of the user changing the mode
// whilst a log is being written, this stores the mode of
// future logs file as set in the profile preferences. See
// also mIsCurrentLogFileInHtmlFormat.
| // To cover the corner case of the user changing the mode | ||
| // whilst a log is being written, this stores the mode of | ||
| // the current log file. See mIsNextLogFileInHtmlFormat. | ||
| bool mIsCurrentLogFileInHtmlFormat; |
There was a problem hiding this comment.
Suggested revised text:
// To cover the corner case of the user changing the mode
// whilst a log is being written, this stores the mode of
// the current log file and is set from
// mIsNextLogFileInHtmlFormat at the point that a log is started.
|
Ok with the phpBB comment stuff going away - it is an idea that is so far down my task list that it is in danger of being compressed so much it might turn into a precious stone... 🔹 😀 <off-topic>b****r is slang from some parts of the UK and when used in conjunction with -up has a meaning in the same general direction as fubar. Despite this Wiktionary entry describing the term as having no sexual overtones - there is actually a linkage of the obscured word to a certain practice that was made legal in mainland Britain only 50 years ago and which could cause offence in some quarters.</off-top> |
SlySven
left a comment
There was a problem hiding this comment.
If it compiles cleanly - and that pointer does not need to be set, then it is OK! 👍
| mpHost->mpMap->mpM = mpMapper->glWidget; | ||
| mpHost->mpMap->mpHost = mpHost; | ||
| mpHost->mpMap->mpMapper = mpMapper; | ||
| mpMapper->mpHost = mpHost; |
There was a problem hiding this comment.
Is this not needed because it is done by the dlgMapper constructor? 😕
| @@ -117,7 +119,6 @@ public slots: | |||
|
|
|||
| GLfloat rotTri, rotQuad; | |||
No description provided.