Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

feature: add close tabs to the right, #822 #875

Merged
merged 5 commits into from Jan 4, 2020
Merged

feature: add close tabs to the right, #822 #875

merged 5 commits into from Jan 4, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jan 2, 2020

Please let me know if this is ok. If something is missing just tell me.
As far as i can see this works just fine.
I have no automated test for this. Is this required?

@mlangkabel
Copy link
Contributor

Thank you for handing in this pull request! The failed CI build is probably not your fault. I'm looking into that right now.

@ghost
Copy link
Author

ghost commented Jan 3, 2020

Did you get a chance to look into it?
Is there anything i could do to help?

@egraether
Copy link
Contributor

I'm looking into it now.

Copy link
Contributor

@egraether egraether left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks fine. Please fix the formatting and white spacing issues. Thanks for implementing!

#include <QtContextMenu.h>
#include <QtGraphicsView.h>
#include <logging.h>
#include <QContextMenuEvent>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow convention:
For Qt headers we don't add '.h' e.g. #include <QContextMenuEvent>
For own headers we use double quotations e.g. #include "QtContextMenu.h"
Qt headers are placed above own headers (see other files)

@@ -76,6 +76,7 @@ private slots:

void legendClicked();


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this change.


void QtTabBar::contextMenuEvent(QContextMenuEvent* event)
{
QtContextMenu menu(event, this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your whole implementation uses the wrong indentation. Please change everything to tabs instead of spaces.

@ghost
Copy link
Author

ghost commented Jan 3, 2020

thanks for the feedback. will make the changes and then turn on to the next issue...

@mlangkabel
Copy link
Contributor

Yes, I managed to fix the CI. So once you made the changes @egraether requested, you can make a new commit and the CI will automatically re-run the checks on your pull request.

@ghost
Copy link
Author

ghost commented Jan 3, 2020

So, i hope this is it :)
Have been struggling with KDevelop. I guess i'll switch back to vim. Much less trouble...

@egraether egraether merged commit 2422e09 into CoatiSoftware:master Jan 4, 2020
@egraether
Copy link
Contributor

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants