Might it make more sense to have D10862 depend on this, rather than the other way around?

Can you redo this patch so it apples cleanly to master?

Joshua, can you provide your email address so we can associate your authorship information with this patch?

In the future, if you upload patches using arc, this will happen automatically. See our documentation: https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches

In D10935#220065, @hindenburg wrote:

Can you redo this patch so it apples cleanly to master?

Done.

In D10935#220147, @ngraham wrote:

Joshua, can you provide your email address so we can associate your authorship information with this patch?

In the future, if you upload patches using arc, this will happen automatically. See our documentation: https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches

Yes, I will definitely try out Arcanist the next time.

joshua(dot)noack(at)gmail(dot)com

Thanks!

FWIW, even after that rebase, I needed to invoke arc with --skip-dependencies to get this patch to apply cleanly, or else arc erroneously tried to also patch in D10862, which is weird, since that already landed. Maybe not landing it with arc made it go mental or something?

Also, +1, works great! I love it. KMessageWidgets are quite nice, and using one here suits Konsole very well.

Thanks for the --skip-dependencies help

I'm not positive adding funcational std::function is worth it for these 2 instances but it is used in KF5 quite a bit.

If the "output has been suspended" is active and that tab is dnd to create another window, the kmessage is not shown and it seems to take an extra Ctrl+q to get the tab to resume.

In D10935#220359, @hindenburg wrote:

If the "output has been suspended" is active and that tab is dnd to create another window, the kmessage is not shown and it seems to take an extra Ctrl+q to get the tab to resume.

Right, since the "suspended mode" is not restored when a new TerminalDisplay is created (i.e. detach). It's not a new bug, so a fix would be unrelated for this revision. But I can fix it separately when I find the time.

@hindenburg if you need any help with using arc land, let me know and I'd be happy to render assistance. But it should be as simple as:

arc patch D10935
git commit --amend --author "Joshua Noack <joshua.noack@gmail.com>"
arc land

My only issue is how to change the commit message - doing 'git commit --amend; arc land' on the previous "read only" patch didn't use the new message.

Hmm, git commit --amend should totally work for modifying the commit message. I know it does work for modifying authorship information (I've used that myself many times).

I'm missing something - arc land shows my changed message but uses the old message

kurthindenburg:src/ (arcpatch-D10935) $ arc land
Landing current branch 'arcpatch-D10935'.
TARGET Landing onto "master", the default target under git.
REMOTE Using remote "origin", the default remote under git.
FETCH Fetching origin/master...
This commit will be landed:

• 45f12a7 Use KMessageWidget to replace suspended QLabel and for locked sesssion.
  
  This branch has revision 'D10935: Konsole: Use KMessageWidget in TerminalDisplay' but you are not the author. Land this revision by jnoack? [y/N] y

Landing revision 'D10935: Konsole: Use KMessageWidget in TerminalDisplay'...
PUSHING Pushing changes to "origin/master".

See here.

arc land does arc amend before landing (to get the "reviewed/accepted by" line from Phab to the local checkout), so any local changes you did to title/summary/test plan/subscribers etc. will be discarded (does not apply to author/committer/patch). Either change the title on Phab, or issue this:

arc amend
git commit --amend
arc diff --edit --verbatim
arc land

Alternatively, if you know what you are doing:

arc amend
git commit --amend
git push

(Untested, but let me know if it fails for you.)

Other apps (dolphin, kmail) use the 'cross on red bg' button in the upper right corner to close the msg.

I think that is much more intuitive and consistent (and shorter) than having a 'close' URL in the ever changing text formulations like 'Please click _here_' or '_Dismiss_' or ...

If you want I can submit a proper bug for this.

In D10935#223341, @ach wrote:

Other apps (dolphin, kmail) use the 'cross on red bg' button in the upper right corner to close the msg.

I think that is much more intuitive and consistent (and shorter) than having a 'close' URL in the ever changing text formulations like 'Please click _here_' or '_Dismiss_' or ...

If you want I can submit a proper bug for this.

I initially tried that, but when the close button has been pressed there's no way to bring back the message widget without creating a new one unless I missed something?
Go readonly -> Dismiss via close button -> Undo readonly -> Make readonly -> no message widget visible since it has been closed and not hidden.

In case it's not hidden, but destroyed, create a new KMessageWidget?
Or maybe the 'close' button can be remapped to trigger a hide instead of destroy?

In D10935#223346, @ach wrote:

In case it's not hidden, but destroyed, create a new KMessageWidget?
Or maybe the 'close' button can be remapped to trigger a hide instead of destroy?

Good question. I'll look into it.

@ach Thanks for bringing this up. Found a way better approach. Will create a new revision for it.