Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Sonntag, den 29.04.2018, 12:59 -0400 schrieb Scott Kostyshak:
> Seems to work very well in a lot of testing I just did.

Many thanks for doing this.

> I've done some testing of weird situations (e.g. a box containing a
> greyed out inset, and putting all of that inside another box and
> putting
> all that in a section). And also, putting that nested thing in a
> float
> caption and then putting that float in a section.
> 
> I'm surprised (and strangely a little disappointed in myself) that I
> was
> not able to break something :) Nice work!

Thanks. I am sure you will be able to proof your ability of breaking
things at another occasion.

I'll commit, then.

Jürgen

> 
> Scott

signature.asc
Description: This is a digitally signed message part

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Sun, Apr 29, 2018 at 08:25:53AM +, Jürgen Spitzmüller wrote:
> Am Dienstag, den 24.04.2018, 12:27 -0400 schrieb Scott Kostyshak:
> > > It's more complicated. You need to pass down runparams.need_protect
> > > to the
> > > diverse cprotect methods. I'll have a look if I find some time.
> > 
> > OK sounds good.
> 
> Here's a patch for you to try. This works for me for boxes, grexed out
> notes and floats in section headings. But I haven't checked hard for
> potential side effects.

Seems to work very well in a lot of testing I just did.

I've done some testing of weird situations (e.g. a box containing a
greyed out inset, and putting all of that inside another box and putting
all that in a section). And also, putting that nested thing in a float
caption and then putting that float in a section.

I'm surprised (and strangely a little disappointed in myself) that I was
not able to break something :) Nice work!

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Dienstag, den 24.04.2018, 12:27 -0400 schrieb Scott Kostyshak:
> > It's more complicated. You need to pass down runparams.need_protect
> > to the
> > diverse cprotect methods. I'll have a look if I find some time.
> 
> OK sounds good.

Here's a patch for you to try. This works for me for boxes, grexed out
notes and floats in section headings. But I haven't checked hard for
potential side effects.

Jürgen

> 
> Scottdiff --git a/lib/layouts/stdinsets.inc b/lib/layouts/stdinsets.inc
index f03b915e7c..c08272284c 100644
--- a/lib/layouts/stdinsets.inc
+++ b/lib/layouts/stdinsets.inc
@@ -508,6 +508,7 @@ InsetLayout Box:Doublebox
 End
 
 InsetLayout Float
+	LaTeXType environment
 	LabelFont
 	  Color   collapsible
 	  SizeSmall
diff --git a/src/Paragraph.cpp b/src/Paragraph.cpp
index 5fdfb157d0..307f8f4549 100644
--- a/src/Paragraph.cpp
+++ b/src/Paragraph.cpp
@@ -1794,7 +1794,9 @@ void Paragraph::write(ostream & os, BufferParams const & bparams,
 void Paragraph::validate(LaTeXFeatures & features) const
 {
 	d->validate(features);
-	if (needsCProtection())
+	bool fragile = features.runparams().moving_arg;
+	fragile |= layout().needprotect;
+	if (needsCProtection(fragile))
 		features.require("cprotect");
 }
 
@@ -3478,7 +3480,7 @@ bool Paragraph::isHardHyphenOrApostrophe(pos_type pos) const
 }
 
 
-bool Paragraph::needsCProtection() const
+bool Paragraph::needsCProtection(bool const fragile) const
 {
 	// first check the layout of the paragraph, but only in insets
 	InsetText const * textinset = inInset().asInsetText();
@@ -3506,7 +3508,7 @@ bool Paragraph::needsCProtection() const
 	// now check whether we have insets that need cprotection
 	pos_type size = d->text_.size();
 	for (pos_type i = 0; i < size; ++i)
-		if (isInset(i) && getInset(i)->needsCProtection(maintext))
+		if (isInset(i) && getInset(i)->needsCProtection(maintext, fragile))
 			return true;
 
 	return false;
diff --git a/src/Paragraph.h b/src/Paragraph.h
index 2a308d6efa..450dfe3265 100644
--- a/src/Paragraph.h
+++ b/src/Paragraph.h
@@ -426,7 +426,7 @@ public:
 	bool isHardHyphenOrApostrophe(pos_type pos) const;
 	/// Return true if this paragraph has verbatim content that needs to be
 	/// protected by \cprotect
-	bool needsCProtection() const;
+	bool needsCProtection(bool const fragile = false) const;
 
 	/// returns true if at least one line break or line separator has been deleted
 	/// at the beginning of the paragraph (either physically or logically)
diff --git a/src/insets/Inset.h b/src/insets/Inset.h
index b6b8db64a5..0f76671ea8 100644
--- a/src/insets/Inset.h
+++ b/src/insets/Inset.h
@@ -595,7 +595,7 @@ public:
 	virtual void rejectChanges() {}
 
 	///
-	virtual bool needsCProtection(bool const) const { return false; }
+	virtual bool needsCProtection(bool const, bool const) const { return false; }
 
 	///
 	virtual ColorCode backgroundColor(PainterInfo const &) const;
diff --git a/src/insets/InsetBox.cpp b/src/insets/InsetBox.cpp
index e543b44fb3..2219c23e37 100644
--- a/src/insets/InsetBox.cpp
+++ b/src/insets/InsetBox.cpp
@@ -201,6 +201,17 @@ bool InsetBox::forcePlainLayout(idx_type) const
 }
 
 
+bool InsetBox::needsCProtection(bool const maintext, bool const fragile) const
+{
+	// We need to cprotect boxes that use minipages as inner box
+	// in fragile context
+	if (fragile && params_.inner_box && !params_.use_parbox && !params_.use_makebox)
+		return true;
+
+	return InsetText::needsCProtection(maintext, fragile);
+}
+
+
 ColorCode InsetBox::backgroundColor(PainterInfo const &) const
 {
 	// we only support background color for 3 types
@@ -336,7 +347,7 @@ void InsetBox::latex(otexstream & os, OutputParams const & runparams) const
 	string separation_string = params_.separation.asLatexString();
 	string shadowsize_string = params_.shadowsize.asLatexString();
 	bool stdwidth = false;
-	string const cprotect = hasCProtectContent() ? "\\cprotect" : string();
+	string const cprotect = hasCProtectContent(runparams.moving_arg) ? "\\cprotect" : string();
 	// in general the overall width of some decorated boxes is wider thean the inner box
 	// we could therefore calculate the real width for all sizes so that if the user wants
 	// e.g. 0.1\columnwidth or 2cm he gets exactly this size
diff --git a/src/insets/InsetBox.h b/src/insets/InsetBox.h
index d5610b746a..e181bd738a 100644
--- a/src/insets/InsetBox.h
+++ b/src/insets/InsetBox.h
@@ -123,6 +123,9 @@ public:
 	///
 	bool forcePlainLayout(idx_type = 0) const;
 	///
+	bool needsCProtection(bool const maintext = false,
+			  bool const fragile = false) const;
+	///
 	bool neverIndent() const { return true; }
 	///
 	bool inheritFont() const { return false; }
diff --git a/src/insets/InsetText.cpp b/src/insets/InsetText.cpp
index 61c4a19b34..dad7e2e248 100644
--- a/src/insets/InsetText.cpp
+++ b/src/insets/InsetText.cpp
@@ -458,7 +458,7 @@ void InsetText::latex(otexstream & os, OutputParams const & runparams) const
 			// FIXME UNICODE
 			// 

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

2018-04-24 3:55 GMT+02:00 Scott Kostyshak :

> I tried the attached patch, but it did not help with the attached .lyx
> example files.
>

It's more complicated. You need to pass down runparams.need_protect to the
diverse cprotect methods. I'll have a look if I find some time.

Jürgen

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Tue, Apr 24, 2018 at 01:55:31AM +, Scott Kostyshak wrote:
> On Mon, Apr 23, 2018 at 02:56:53PM +, Jürgen Spitzmüller wrote:
> > 2018-04-23 16:38 GMT+02:00 Scott Kostyshak :
> > 
> > > That could be. I really don't know. Is there a LyX method to check if
> > > the context is fragile? Or should I hardcode the names of the
> > > environments that seem to need it (from testing)?
> > >
> > 
> > The NeedProtect flag of layouts.
> 
> I tried the attached patch, but it did not help with the attached .lyx
> example files.

Oops, patch now attached.

Scott

> > > Alternatively, I'm also fine to stop working on this. I thought there
> > > was some low-hanging fruit, but since I don't understand the
> > > complexities, it might be better to leave it.
> > >
> > 
> > Well, I am amazed how many things seem to be possible now (which I used to
> > think could never be solved), so I think you should move on.
> 
> OK good to know.
> 
> Scott





diff --git a/src/insets/InsetText.cpp b/src/insets/InsetText.cpp
index 61c4a19..5d70dc7 100644
--- a/src/insets/InsetText.cpp
+++ b/src/insets/InsetText.cpp
@@ -1093,14 +1093,15 @@ bool InsetText::needsCProtection(bool const maintext) const
 	if (hasCProtectContent())
 		return true;
 
-	if (!getLayout().needsCProtect())
-		return false;
-
 	// Environments and "no latex" types (e.g., knitr chunks)
 	// need cprotection regardless the content
-	if (!maintext && getLayout().latextype() != InsetLayout::COMMAND)
+	if (!maintext && getLayout().latextype() != InsetLayout::COMMAND &&
+	getLayout().isNeedProtect())
 		return true;
 
+	if (!getLayout().needsCProtect())
+		return false;
+
 	// Commands need cprotection if they contain specific chars
 	int const nchars_escape = 9;
 	static char_type const chars_escape[nchars_escape] = {


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Mon, Apr 23, 2018 at 02:56:53PM +, Jürgen Spitzmüller wrote:
> 2018-04-23 16:38 GMT+02:00 Scott Kostyshak :
> 
> > That could be. I really don't know. Is there a LyX method to check if
> > the context is fragile? Or should I hardcode the names of the
> > environments that seem to need it (from testing)?
> >
> 
> The NeedProtect flag of layouts.

I tried the attached patch, but it did not help with the attached .lyx
example files.

> > Alternatively, I'm also fine to stop working on this. I thought there
> > was some low-hanging fruit, but since I don't understand the
> > complexities, it might be better to leave it.
> >
> 
> Well, I am amazed how many things seem to be possible now (which I used to
> think could never be solved), so I think you should move on.

OK good to know.

Scott


environmentsInCommands.lyx
Description: application/lyx


figureInSection.lyx
Description: application/lyx


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

2018-04-23 16:38 GMT+02:00 Scott Kostyshak :

> That could be. I really don't know. Is there a LyX method to check if
> the context is fragile? Or should I hardcode the names of the
> environments that seem to need it (from testing)?
>

The NeedProtect flag of layouts.


>
> Alternatively, I'm also fine to stop working on this. I thought there
> was some low-hanging fruit, but since I don't understand the
> complexities, it might be better to leave it.
>

Well, I am amazed how many things seem to be possible now (which I used to
think could never be solved), so I think you should move on.

Jürgen


>
> Thanks for the help,
>
> Scott
>

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

2018-04-23 16:38 GMT+02:00 Scott Kostyshak :

> I misunderstood the comment
>
>// Environments and "no latex" types (e.g., knitr chunks)
>// need cprotection regardless the content
>
> Reading it again, it was my mistake to interpret it as "every command
> including an environment should be cprotected".
>

For that matter: Environments and "no latex" types that have the
NeedCProtect flag.

Jürgen

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Mon, Apr 23, 2018 at 08:19:12AM +, Jürgen Spitzmüller wrote:
> Am Montag, den 23.04.2018, 09:30 +0200 schrieb Jürgen Spitzmüller:
> > AFAIU this would simply overshoot the cprotecting. It would cprotect
> > each single command that has an environment in its argument, but
> > AFAIU
> > only some commands need the cprotecting (I suppose those who are not
> > \long).
> 
> Without having tested it, but is it possible that the real problem is
> that we need to cprotect environments (only) in fragile contexts (such
> as section headings and captions)?

That could be. I really don't know. Is there a LyX method to check if
the context is fragile? Or should I hardcode the names of the
environments that seem to need it (from testing)?

Alternatively, I'm also fine to stop working on this. I thought there
was some low-hanging fruit, but since I don't understand the
complexities, it might be better to leave it.

Thanks for the help,

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Mon, Apr 23, 2018 at 07:30:01AM +, Jürgen Spitzmüller wrote:
> Am Sonntag, den 22.04.2018, 15:48 -0400 schrieb Scott Kostyshak:
> > Could you please give an example (e.g. steps to reproduce a problem
> > with the patch)?
> 
> AFAIU this would simply overshoot the cprotecting. It would cprotect
> each single command that has an environment in its argument, but AFAIU
> only some commands need the cprotecting (I suppose those who are not
> \long).

Ah I see. I had misunderstood your previous comment to be literally "any
command". Indeed, the purpose of the patch is "any command that has an
environment".

> Also this strikes me like a strange use of \cprotect. Where did you get
> the advise to use it in these cases from?

I misunderstood the comment

   // Environments and "no latex" types (e.g., knitr chunks)
   // need cprotection regardless the content

Reading it again, it was my mistake to interpret it as "every command
including an environment should be cprotected".

I've only quickly skimmed the cprotect manual, but I did not find any
advice for general situations. The manual is very much focused on the
case of verbatim.

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Montag, den 23.04.2018, 09:30 +0200 schrieb Jürgen Spitzmüller:
> AFAIU this would simply overshoot the cprotecting. It would cprotect
> each single command that has an environment in its argument, but
> AFAIU
> only some commands need the cprotecting (I suppose those who are not
> \long).

Without having tested it, but is it possible that the real problem is
that we need to cprotect environments (only) in fragile contexts (such
as section headings and captions)?

Jürgen

signature.asc
Description: This is a digitally signed message part

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Sonntag, den 22.04.2018, 15:48 -0400 schrieb Scott Kostyshak:
> Could you please give an example (e.g. steps to reproduce a problem
> with the patch)?

AFAIU this would simply overshoot the cprotecting. It would cprotect
each single command that has an environment in its argument, but AFAIU
only some commands need the cprotecting (I suppose those who are not
\long).

Also this strikes me like a strange use of \cprotect. Where did you get
the advise to use it in these cases from?

Jürgen

> 
> Scott

signature.asc
Description: This is a digitally signed message part

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Sun, Apr 22, 2018 at 05:57:15PM +, Jürgen Spitzmüller wrote:
> Am Sonntag, den 22.04.2018, 13:23 -0400 schrieb Scott Kostyshak:
> > What is the !getLayout().needsCProtect() check for? Does it need to
> > go
> > before the environment check, or is the attached patch OK?
> 
> This looks wrong. This would mean that any command would be cprotected,
> notwithstanding the contents.

Could you please give an example (e.g. steps to reproduce a problem
with the patch)?

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Sonntag, den 22.04.2018, 13:23 -0400 schrieb Scott Kostyshak:
> What is the !getLayout().needsCProtect() check for? Does it need to
> go
> before the environment check, or is the attached patch OK?

This looks wrong. This would mean that any command would be cprotected,
notwithstanding the contents.

Jürgen

> 
> Scott

signature.asc
Description: This is a digitally signed message part

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Fri, Apr 13, 2018 at 03:53:28PM +, Juergen Spitzmueller wrote:

> +bool InsetText::needsCProtection() const
> +{
> + if (!getLayout().needsCProtect())
> + return false;
> +
> + // Environments need cprotection regardless the content
> + if (getLayout().latextype() == InsetLayout::ENVIRONMENT)
> + return true;

What is the !getLayout().needsCProtect() check for? Does it need to go
before the environment check, or is the attached patch OK?

Scott
From 4554a76ac3edf79b9c04c3a1791ac48a79c533d9 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak 
Date: Sun, 22 Apr 2018 13:10:26 -0400
Subject: [PATCH] Fix cprotection of environments in commands

Commands that contain any environment should be cprotected. This
check should be done before the check for
getLayout().needsCProtect().

This commit fixes compilation of, e.g., boxes and floats in section
titles.
---
 src/insets/InsetText.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/insets/InsetText.cpp b/src/insets/InsetText.cpp
index b38a1dc..ce00da7 100644
--- a/src/insets/InsetText.cpp
+++ b/src/insets/InsetText.cpp
@@ -1093,14 +1093,14 @@ bool InsetText::needsCProtection() const
 	if (hasCProtectContent())
 		return true;
 
-	if (!getLayout().needsCProtect())
-		return false;
-
 	// Environments and "no latex" types (e.g., knitr chunks)
 	// need cprotection regardless the content
 	if (getLayout().latextype() != InsetLayout::COMMAND)
 		return true;
 
+	if (!getLayout().needsCProtect())
+		return false;
+
 	// Commands need cprotection if they contain specific chars
 	int const nchars_escape = 9;
 	static char_type const chars_escape[nchars_escape] = {
-- 
2.7.4



signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Thu, Apr 19, 2018 at 06:49:31AM +, Jürgen Spitzmüller wrote:
> Am Mittwoch, den 18.04.2018, 15:19 -0400 schrieb Scott Kostyshak:

> > Any objection to put this patch in?
> 
> None from me. Thanks for investigating.

Pushed to master at 80fe0dbe.

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Mittwoch, den 18.04.2018, 15:19 -0400 schrieb Scott Kostyshak:
> On Mon, Apr 16, 2018 at 04:03:44PM +, Scott Kostyshak wrote:
> 
> > I see. Thanks for the explanation. OK I will test whether they are
> > needed with the example I gave or with the knitr example file. If
> > it is
> > not needed for both, I will remove it for that type of box. Let me
> > know
> > if there is any other test case that you think would be good to
> > check.
> 
> Attached is a patch.
> 
> I first tested compilation of all box types for both the knitr manual
> and for the verbatim example in current master. All compilations
> succeeded. I then removed \cprotect for all box types, and only the
> simple frame failed.
> 
> I then tested that with the attached patch, all compilations succeed.
> (Note that I tested all compilations with formats pdf, pdf2, pdf3,
> pdf4,
> and pdf5, and not surprisingly all formats give the same result).
> 
> Any objection to put this patch in?

None from me. Thanks for investigating.

Jürgen

> 
> Scott

signature.asc
Description: This is a digitally signed message part

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Mon, Apr 16, 2018 at 04:03:44PM +, Scott Kostyshak wrote:

> I see. Thanks for the explanation. OK I will test whether they are
> needed with the example I gave or with the knitr example file. If it is
> not needed for both, I will remove it for that type of box. Let me know
> if there is any other test case that you think would be good to check.

Attached is a patch.

I first tested compilation of all box types for both the knitr manual
and for the verbatim example in current master. All compilations
succeeded. I then removed \cprotect for all box types, and only the
simple frame failed.

I then tested that with the attached patch, all compilations succeed.
(Note that I tested all compilations with formats pdf, pdf2, pdf3, pdf4,
and pdf5, and not surprisingly all formats give the same result).

Any objection to put this patch in?

Scott
From 7c018696f1d5a6e2dc257010edbcd09e5cc8c827 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak 
Date: Wed, 18 Apr 2018 15:11:55 -0400
Subject: [PATCH] Use \cprotect only for "simple frame" boxes

Compilation succeeds with verbatim environments for the other types
of boxes without \cprotect.
---
 src/insets/InsetBox.cpp | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/insets/InsetBox.cpp b/src/insets/InsetBox.cpp
index 7dc5ed3..e543b44 100644
--- a/src/insets/InsetBox.cpp
+++ b/src/insets/InsetBox.cpp
@@ -407,10 +407,10 @@ void InsetBox::latex(otexstream & os, OutputParams const & runparams) const
 			os << "{\\fboxsep " << from_ascii(separation_string);
 		if (!params_.inner_box && !width_string.empty()) {
 			if (params_.framecolor != "black" || params_.backgroundcolor != "none") {
-os << cprotect << "\\fcolorbox{" << params_.framecolor << "}{" << params_.backgroundcolor << "}{";
+os << "\\fcolorbox{" << params_.framecolor << "}{" << params_.backgroundcolor << "}{";
 os << "\\makebox";
 			} else
-os << cprotect << "\\framebox";
+os << "\\framebox";
 			// Special widths, see usrguide sec. 3.5
 			// FIXME UNICODE
 			if (params_.special != "none") {
@@ -425,7 +425,7 @@ void InsetBox::latex(otexstream & os, OutputParams const & runparams) const
 os << "[" << params_.hor_pos << "]";
 		} else {
 			if (params_.framecolor != "black" || params_.backgroundcolor != "none")
-os << cprotect << "\\fcolorbox{" << params_.framecolor << "}{" << params_.backgroundcolor << "}";
+os << "\\fcolorbox{" << params_.framecolor << "}{" << params_.backgroundcolor << "}";
 			else
 os << cprotect << "\\fbox";
 		}
@@ -434,12 +434,12 @@ void InsetBox::latex(otexstream & os, OutputParams const & runparams) const
 	case ovalbox:
 		if (!separation_string.empty() && separation_string != defaultSep)
 			os << "{\\fboxsep " << from_ascii(separation_string);
-		os << cprotect << "\\ovalbox{";
+		os << "\\ovalbox{";
 		break;
 	case Ovalbox:
 		if (!separation_string.empty() && separation_string != defaultSep)
 			os << "{\\fboxsep " << from_ascii(separation_string);
-		os << cprotect << "\\Ovalbox{";
+		os << "\\Ovalbox{";
 		break;
 	case Shadowbox:
 		if (thickness_string != defaultThick) {
@@ -461,7 +461,7 @@ void InsetBox::latex(otexstream & os, OutputParams const & runparams) const
 && separation_string == defaultSep
 && thickness_string == defaultThick)
 os << "{\\shadowsize " << from_ascii(shadowsize_string);
-		os << cprotect << "\\shadowbox{";
+		os << "\\shadowbox{";
 		break;
 	case Shaded:
 		// must be set later because e.g. the width settings only work when
@@ -475,7 +475,7 @@ void InsetBox::latex(otexstream & os, OutputParams const & runparams) const
 		}
 		if (separation_string != defaultSep && thickness_string == defaultThick)
 			os << "{\\fboxsep " << from_ascii(separation_string);
-		os << cprotect << "\\doublebox{";
+		os << "\\doublebox{";
 		break;
 	}
 
-- 
2.7.4



signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Mon, Apr 16, 2018 at 03:52:38PM +, Jürgen Spitzmüller wrote:
> 2018-04-16 17:43 GMT+02:00 Scott Kostyshak :
> 
> > I don't mind doing it, but do you agree that it should be done?
> >
> 
> If we are certain about it.
> 
> 
> > I have no knowledge of \cprotect, so I don't know if e.g. in the case of
> > double frame, although it is not needed for the example file I provided,
> > perhaps it is needed for another situation? I also have no idea if there
> > is any disadvantage to using \cprotect when it is not needed. Perhaps
> > you are suggesting that there is no disadvantage so it is not worth the
> > 30 minutes to remove (and test) it?
> >
> 
> I also can only say that per trial and error. For instance, it is not
> documented in the framed manual that verbatim content is supported, I got
> that information from some stackexchange post.
> Likewise, I have no idea why parbox cannot be cprotected.
> 
> In general, I think that it does not harm doing a \cprotect where it is not
> necessary (we have to do that in some cases); but on the other hand, if we
> are sure that something works without, there is no need to do it.
> 
> So what I wanted to say is that you can go ahead and remove the respective
> "cprotect" strings before the specific box calls if your testing reveals
> they are not needed. I don't have any more expertise than you here. I would
> need to sit down and test as well (and if you want to d it instead, I would
> be actually rather grateful).

I see. Thanks for the explanation. OK I will test whether they are
needed with the example I gave or with the knitr example file. If it is
not needed for both, I will remove it for that type of box. Let me know
if there is any other test case that you think would be good to check.

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

2018-04-16 17:43 GMT+02:00 Scott Kostyshak :

> I don't mind doing it, but do you agree that it should be done?
>

If we are certain about it.


> I have no knowledge of \cprotect, so I don't know if e.g. in the case of
> double frame, although it is not needed for the example file I provided,
> perhaps it is needed for another situation? I also have no idea if there
> is any disadvantage to using \cprotect when it is not needed. Perhaps
> you are suggesting that there is no disadvantage so it is not worth the
> 30 minutes to remove (and test) it?
>

I also can only say that per trial and error. For instance, it is not
documented in the framed manual that verbatim content is supported, I got
that information from some stackexchange post.
Likewise, I have no idea why parbox cannot be cprotected.

In general, I think that it does not harm doing a \cprotect where it is not
necessary (we have to do that in some cases); but on the other hand, if we
are sure that something works without, there is no need to do it.

So what I wanted to say is that you can go ahead and remove the respective
"cprotect" strings before the specific box calls if your testing reveals
they are not needed. I don't have any more expertise than you here. I would
need to sit down and test as well (and if you want to d it instead, I would
be actually rather grateful).

Jürgen


>
> Scott
>

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Mon, Apr 16, 2018 at 07:56:15AM +, Jürgen Spitzmüller wrote:
> Am Sonntag, den 15.04.2018, 13:47 -0400 schrieb Scott Kostyshak:
> > I see. So in this case should we remove \cprotect for situations that
> > don't need it?
> 
> Feel free to do that.

I don't mind doing it, but do you agree that it should be done?

I have no knowledge of \cprotect, so I don't know if e.g. in the case of
double frame, although it is not needed for the example file I provided,
perhaps it is needed for another situation? I also have no idea if there
is any disadvantage to using \cprotect when it is not needed. Perhaps
you are suggesting that there is no disadvantage so it is not worth the
30 minutes to remove (and test) it?

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Sonntag, den 15.04.2018, 13:47 -0400 schrieb Scott Kostyshak:
> I see. So in this case should we remove \cprotect for situations that
> don't need it?

Feel free to do that.

Jürgen

> 
> Scott

signature.asc
Description: This is a digitally signed message part

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Sun, Apr 15, 2018 at 04:18:11PM +, Jürgen Spitzmüller wrote:
> Am Sonntag, den 15.04.2018, 10:45 -0400 schrieb Scott Kostyshak:
> > I think I was unclear. I mean that before your commit, if you put a
> > box
> > around a verbatim, it worked unless the box was a "simple frame". To
> > see
> > this, open the file simpleFrame_verbatim.lyx that I sent before with
> > e.g. LyX 2.3.0 and change from "simple frame" to "double frame". It
> > compiled without error.
> 
> This seems to be a feature of the framed package:
> https://tex.stackexchange.com/a/6311/19291

I see. So in this case should we remove \cprotect for situations that
don't need it?

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Sonntag, den 15.04.2018, 10:45 -0400 schrieb Scott Kostyshak:
> I think I was unclear. I mean that before your commit, if you put a
> box
> around a verbatim, it worked unless the box was a "simple frame". To
> see
> this, open the file simpleFrame_verbatim.lyx that I sent before with
> e.g. LyX 2.3.0 and change from "simple frame" to "double frame". It
> compiled without error.

This seems to be a feature of the framed package:
https://tex.stackexchange.com/a/6311/19291

> > Please test with the attached patch after my recent commits. I did
> > not
> > commit the patch since I cannot test whether it works (no R
> > installed
> > here).
> 
> Tested and works well. Thank you!

Committed.

Jürgen

> 
> Scott

signature.asc
Description: This is a digitally signed message part

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Sun, Apr 15, 2018 at 10:35:32AM +, Jürgen Spitzmüller wrote:
> Am Samstag, den 14.04.2018, 13:15 -0400 schrieb Scott Kostyshak:
> > Did we need to add \cprotect also for the other box modes? For the
> > example file, all of the options besides simple frame compiled
> > without
> > error. Was that just by luck?
> 
> No, because I added the calls. Since the Box latex routine is really
> ugly, chances are high, though, that I missed cases. So thanks for
> checking.

I think I was unclear. I mean that before your commit, if you put a box
around a verbatim, it worked unless the box was a "simple frame". To see
this, open the file simpleFrame_verbatim.lyx that I sent before with
e.g. LyX 2.3.0 and change from "simple frame" to "double frame". It
compiled without error.

> The reason was that knitr chunk has the LaTeXType "none" which was
> treated equivalently to "command" (which only needs cprotect for
> specific content), rather than "environment" (which needs cprotect
> always). Thus cprotect would have chimed in if you entered one of the
> crucial chars in the chunk (e.g., #).

Ah that makes sense.

> Please test with the attached patch after my recent commits. I did not
> commit the patch since I cannot test whether it works (no R installed
> here).

Tested and works well. Thank you!

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Samstag, den 14.04.2018, 13:15 -0400 schrieb Scott Kostyshak:
> Did we need to add \cprotect also for the other box modes? For the
> example file, all of the options besides simple frame compiled
> without
> error. Was that just by luck?

No, because I added the calls. Since the Box latex routine is really
ugly, chances are high, though, that I missed cases. So thanks for
checking.

> > > This issue also affects putting a simple frame around a knitr
> > > chunk,
> > > because knitr uses verbatim.
> > 
> > Try if adding
> > 
> > NeedsCProtect 1
> > to the respective knitr layouts helps.
> 
> It does not seem to change LaTeX output for me (I used
> "NeedsCProtect").
> Perhaps it is because the LaTeX code has also a minipage?
> 
> \fbox{\begin{minipage}[t]{

The reason was that knitr chunk has the LaTeXType "none" which was
treated equivalently to "command" (which only needs cprotect for
specific content), rather than "environment" (which needs cprotect
always). Thus cprotect would have chimed in if you entered one of the
crucial chars in the chunk (e.g., #).

Please test with the attached patch after my recent commits. I did not
commit the patch since I cannot test whether it works (no R installed
here).

Jürgendiff --git a/lib/layouts/litinsets.inc b/lib/layouts/litinsets.inc
index 11dd2d16af..7e1e9f00d7 100644
--- a/lib/layouts/litinsets.inc
+++ b/lib/layouts/litinsets.inc
@@ -51,4 +51,5 @@ InsetLayout "Flex:Chunk"
 EndArgument
 ResetsFontfalse
 ForceOwnlines true
+NeedCProtect  true
 End


signature.asc
Description: This is a digitally signed message part

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Sat, Apr 14, 2018 at 11:01:04AM +, Jürgen Spitzmüller wrote:
> Am Freitag, den 13.04.2018, 22:18 -0400 schrieb Scott Kostyshak:
> > Can we add support for a verbatim inside a simple frame? See the
> > attached, simpleFrame_verbatim.lyx.
> 
> Done. Note that not all cases work (for instance, I don't know how to
> \cprotect a parbox).

This works well. Thank you!

Did we need to add \cprotect also for the other box modes? For the
example file, all of the options besides simple frame compiled without
error. Was that just by luck?

> > This issue also affects putting a simple frame around a knitr chunk,
> > because knitr uses verbatim.
> 
> Try if adding
> 
> NeedsCProtect 1
> to the respective knitr layouts helps.

It does not seem to change LaTeX output for me (I used "NeedsCProtect").
Perhaps it is because the LaTeX code has also a minipage?

\fbox{\begin{minipage}[t]{

To test that the LaTeX is not changed, open examples/knitr.lyx and put a
simple frame around the chunk inset. Attached is the diff containing the
change of adding a box and the change of adding NeedCProtect.

Scott
diff --git a/lib/examples/knitr.lyx b/lib/examples/knitr.lyx
index 49cc262..fd3c8b7 100644
--- a/lib/examples/knitr.lyx
+++ b/lib/examples/knitr.lyx
@@ -1,5 +1,5 @@
-#LyX 2.3 created this file. For more info see http://www.lyx.org/
-\lyxformat 544
+#LyX 2.4 created this file. For more info see https://www.lyx.org/
+\lyxformat 547
 \begin_document
 \begin_header
 \save_transient_properties true
@@ -242,6 +242,25 @@ knitr
 \end_layout
 
 \begin_layout Standard
+\begin_inset Box Boxed
+position "t"
+hor_pos "c"
+has_inner_box 1
+inner_pos "t"
+use_parbox 0
+use_makebox 0
+width "100col%"
+special "none"
+height "1in"
+height_special "totalheight"
+thickness "0.4pt"
+separation "3pt"
+shadowsize "4pt"
+framecolor "black"
+backgroundcolor "none"
+status open
+
+\begin_layout Plain Layout
 \begin_inset Flex Chunk
 status open
 
@@ -280,6 +299,11 @@ summary(lm(y~x, data=df))
 
 \end_layout
 
+\end_inset
+
+
+\end_layout
+
 \begin_layout Standard
 Please contact the package author in case of any problems.
 \end_layout
diff --git a/lib/layouts/litinsets.inc b/lib/layouts/litinsets.inc
index 1a72fed..96b6794 100644
--- a/lib/layouts/litinsets.inc
+++ b/lib/layouts/litinsets.inc
@@ -51,4 +51,5 @@ InsetLayout "Flex:Chunk"
 EndArgument
 ResetsFontfalse
 ForceOwnlines true
+NeedCProtect  1
 End


signature.asc
Description: PGP signature

Re: [LyX/master] Add basic support for cprotect

[Jürgen Spitzmüller]

Am Freitag, den 13.04.2018, 22:18 -0400 schrieb Scott Kostyshak:
> Can we add support for a verbatim inside a simple frame? See the
> attached, simpleFrame_verbatim.lyx.

Done. Note that not all cases work (for instance, I don't know how to
\cprotect a parbox).

> This issue also affects putting a simple frame around a knitr chunk,
> because knitr uses verbatim.

Try if adding

NeedsCProtect 1

to the respective knitr layouts helps.

Jürgen

> 
> Scott

signature.asc
Description: This is a digitally signed message part

Re: [LyX/master] Add basic support for cprotect

[Scott Kostyshak]

On Fri, Apr 13, 2018 at 03:53:28PM +, Juergen Spitzmueller wrote:

> Note that the implementation is still rather basic and might need
> extension for other cases.

Can we add support for a verbatim inside a simple frame? See the
attached, simpleFrame_verbatim.lyx.

Also see the attached file, cprotected.tex, which shows that \cprotect
works.

This issue also affects putting a simple frame around a knitr chunk,
because knitr uses verbatim.

Scott
%% LyX 2.2.1 created this file.  For more info, see http://www.lyx.org/.
%% Do not edit unless you really know what you are doing.
\documentclass[english]{article}
\usepackage[T1]{fontenc}
\usepackage[latin9]{inputenc}
\usepackage{fancybox}
\usepackage{calc}
\usepackage{babel}
\usepackage{cprotect}
\begin{document}
\noindent\cprotect\fbox{\begin{minipage}[t]{1\columnwidth - 2\fboxsep - 2\fboxrule}%
\begin{verbatim}
hello
\end{verbatim}
%
\end{minipage}}
\end{document}


simpleFrame_verbatim.lyx
Description: application/lyx


signature.asc
Description: PGP signature