Differential D7847 Diff 20181 src/panels/terminal/terminalpanel.h

Changeset View

Standalone View

View Options

src/panels/terminal/terminalpanel.h

Context not available.
48 TerminalPanel(QWidget* parent = 0); 48​ TerminalPanel(QWidget* parent = 0);
elvisangelaccioUnsubmitted
Done

Why public? This is only used in a private method.
Also, we are using enum class for new enums, so we should do it also here for consistency.

elvisangelaccio: Why `public`? This is only used in a private method. Also, we are using `enum class` for new…
49 virtual ~TerminalPanel(); 49​ virtual ~TerminalPanel();
elvisangelaccioUnsubmitted
Done

I'd use AddToHistory and SkipHistory.
Not sure about the name of the enum itself, maybe HistoryPolicy ?

elvisangelaccio: I'd use `AddToHistory` and `SkipHistory`. Not sure about the name of the enum itself, maybe…
martonmiklosAuthorUnsubmitted
Done

Thanks for the review, I agree and I have updated the requested changes.

martonmiklos: Thanks for the review, I agree and I have updated the requested changes.
50 50​
51​ void goHome();
52​ QString currentWorkingDirectory();
53​
51public slots: 54​public slots:
52 void terminalExited(); 55​ void terminalExited();
53 void dockVisibilityChanged(); 56​ void dockVisibilityChanged();
Context not available.
71 74​
72private: 75​private:
73 void changeDir(const QUrl& url); 76​ void changeDir(const QUrl& url);
74 void sendCdToTerminal(const QString& path); 77​ void sendCdToTerminal(const QString& path, bool addToHistory = true);
elvisangelaccioUnsubmitted
Done

boolean arguments are a bad practice (bad readability of code), please use an enum instead.

elvisangelaccio: boolean arguments are a bad practice (bad readability of code), please use an enum instead.
75 78​
76private: 79​private:
77 bool m_clearTerminal; 80​ bool m_clearTerminal;
Context not available.