[prev in list] [next in list] [prev in thread] [next in thread]
List: konsole-devel
Subject: Re: Review Request 129647: Add --nofork as compatibility alias for --separate
From: Kurt Hindenburg <kurt.hindenburg () gmail ! com>
Date: 2016-12-17 15:43:53
Message-ID: 20161217154353.4515.46038 () mimi ! kde ! org
[Download RAW message or body]
--===============6867327946005237176==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
> On Dec. 17, 2016, 12:08 p.m., Martin Tobias Holmedahl Sandsmark wrote:
> > looks good to me.
> >
> > but reminds me that I don't like string literals being hardcoded in different \
> > places.
>
> Harald Sitter wrote:
> lol, I thought the same thing when I made the patch ^^
>
> main() should get refactored to actually use the option objects of the Application \
> at some point
I'm not sure you can add strings in 16.12 - go ahead if you think so and to master
- Kurt
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129647/#review101479
-----------------------------------------------------------
On Dec. 13, 2016, 8:21 a.m., Harald Sitter wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129647/
> -----------------------------------------------------------
>
> (Updated Dec. 13, 2016, 8:21 a.m.)
>
>
> Review request for Konsole.
>
>
> Repository: konsole
>
>
> Description
> -------
>
> In previous incarnations of kuniqueapplication it used to inject a common
> command option --nofork which is meant to bypass single-instance behavior.
> Given that konsole can and is being invoked from scripts they may well want
> to ensure that the fork they created is the actual instance of konsole.
> i.e. to monitor return values and life time
>
> Presently, since the options are divergent between konsole4 and konsole5,
> scripts are either incompatible with older konsoles or with newer konsoles.
> To make life easier for everyone add an compat alias --nofork, which
> behaves exactly like separate.
>
> (this unbreaks steam, which is a notable recent offender of falling into
> this particular trap)
>
>
> Diffs
> -----
>
> src/Application.cpp 5b352ec9b10899b44abbd6ee1609422ef1434620
> src/main.cpp 26d3da990b6aee564c0c9a237a56a65b1d372508
>
> Diff: https://git.reviewboard.kde.org/r/129647/diff/
>
>
> Testing
> -------
>
> builds + noforks + separates
>
> poor steam also is happily opening a konsole now
>
>
> Thanks,
>
> Harald Sitter
>
>
--===============6867327946005237176==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/129647/">https://git.reviewboard.kde.org/r/129647/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On December 17th, 2016, 12:08 p.m. UTC, <b>Martin \
Tobias Holmedahl Sandsmark</b> wrote:</p> <blockquote style="margin-left: 1em; \
border-left: 2px solid #d0d0d0; padding-left: 10px;"> <pre style="white-space: \
pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">looks good to me.</p> \
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">but reminds me that I don't like string literals being \
hardcoded in different places.</p></pre> </blockquote>
<p>On December 17th, 2016, 12:12 p.m. UTC, <b>Harald Sitter</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">lol, \
I thought the same thing when I made the patch ^^</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">main() should get refactored to actually use the option objects of the \
Application at some point</p></pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm \
not sure you can add strings in 16.12 - go ahead if you think so and to \
master</p></pre> <br />
<p>- Kurt</p>
<br />
<p>On December 13th, 2016, 8:21 a.m. UTC, Harald Sitter wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Konsole.</div>
<div>By Harald Sitter.</div>
<p style="color: grey;"><i>Updated Dec. 13, 2016, 8:21 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
konsole
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">In previous incarnations of kuniqueapplication it used to inject a \
common command option --nofork which is meant to bypass single-instance behavior.
Given that konsole can and is being invoked from scripts they may well want
to ensure that the fork they created is the actual instance of konsole.
i.e. to monitor return values and life time
Presently, since the options are divergent between konsole4 and konsole5,
scripts are either incompatible with older konsoles or with newer konsoles.
To make life easier for everyone add an compat alias --nofork, which
behaves exactly like separate.
(this unbreaks steam, which is a notable recent offender of falling into
this particular trap)</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">builds + noforks + separates</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">poor \
steam also is happily opening a konsole now</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/Application.cpp <span style="color: \
grey">(5b352ec9b10899b44abbd6ee1609422ef1434620)</span></li>
<li>src/main.cpp <span style="color: \
grey">(26d3da990b6aee564c0c9a237a56a65b1d372508)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129647/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============6867327946005237176==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic