In D17374#371839, @thsurrel wrote:

I don't really know how to test this in the embedded terminal in Kate for example. Could someone provide some guidance ?

thanks

I'll see if I find anything after testing

ViewContainer.cpp:133:44: warning: Missing emit keyword on signal call Konsole::TabbedViewContainer::newViewWithProfileRequest

Nice catch thanks

Thanks

This is really good - I noticed that once you detach/close views, the menus View->Close/Expand/Shrink are still active (they don't do anything when selected).

I've noticed a few issues

1. detach split tabs maintain the same size (ie not the full window) but the size the tab was - is this expected behavior?
2. close one side of the view causing the menus to not have all the menus until switching tabs
3. opening a new tab on the non-focus side will not update the tab text (ie should be "src:zsh", it is ":" until you click on tab).

Ok let me look at this - I notice if you double-click on an empty spot in the non-focused view's tabbar, the new tab is created in the focused view. Which is a bit confusing.

w/o the Konsole reviewer, it doesn't show up on https://phabricator.kde.org/project/37/item/view/637/ which is what I use

OK so this is for when 'Use Line characters contained in in Font' is disabled. I see a big improvement then.

Of course thanks

Wow, that's impressive. Thanks a lot for working on this - let me look at it further.

Ok, I think I/someone read the docs and did it that way from the start - I wonder what the Qt docs thinks is a large file.

same issue here - can you fix?

The arc patch is moving files - can you re-upload?

Ok if you want - I thought you might be still working on it

Looks fine - thanks

Yes, I was actually surprised I managed to get it all to run. Are there official docs on how to setup an ci image and run applications? If not, it would be helpful.

In D16919#364934, @bcooksley wrote:

Yes, that is basically what the CI does so should be a pretty good method of reproducing it's results.
Did you run "make tests" / "ctest" or did you use the run-tests.py script to execute the unit tests?

In D16919#364587, @bcooksley wrote:

When you did your reproduction attempt @hindenburg did you use the binaries the CI system itself uses, or did you build Frameworks, etc yourself?

I manage to build the docker kdeorg/ci-suse-qt59 image - and of course the tests pass - so I'm not sure how to help now. I've already disabled this test again to avoid the sysadmins having more work to do.

Thanks

In D16837#364070, @EspadaV8 wrote:

@ngraham Is there anyway to progress this and to decide if this change (or something like it) could be merged into Konsole? I spoke briefly with @hein on the #kde-devel channel on IRC and they didn't see any reason why Yakuake features couldn't be merged into Konsole itself (just that no one has done it before). They did mention that they'd like to see the Konsole KPart functionality improved but I'm not in a position to be able to know exactly what would need to be done to implement those changes.

If this revision isn't okay then I'd be happy to make any suggestions that you have to help get these features directly into Konsole.

This throws ASAN errors

The change appears to be so slight as to be unnoticeable. Only when I change the 48 to 148 can I actually see changes. I would suggest following up on your comment about not just checking for pure black and then also make the change noticeable.

I think the icons are correct as-is, though their may be some other issue w/ the themes as detailed in the bug report. The 'Initial Dir' is asking for a folder so folder-open seems correct. The "User user-defined stylesheet' is asking for a file so the document-open seems OK.

OK the boxes look better w/ patch regardless

yea - feel free to commit any changes related to this yourself if you want.

Thanks so much for looking into this.