Re: [LyX/master] Add accelerator

[Scott Kostyshak]

On Tue, Jun 13, 2017 at 12:02:09PM +0200, Enrico Forestieri wrote:

> Richard, this is a different issue. The user can always set -shell-escape
> and we can't prevent him from doing that. If the user is bothered from
> having to set the option and then delete it again, it may happen that
> he leaves it in place, giving rise to all security risks this entails.
> We cannot bury our head in the sand and blame the user for that.
> Rather, we should try to protect him by setting the option only when
> it is really needed, in an ephemeral way. This does not happen behind
> the user's back and he is warned of that, hence he can properly act.

Thanks for this explanation, Enrico. My personal opinion is that I agree
with what you said and with your patch. My release manager opinion is
that there might be some that disagree and we should hear from them
first. From what I understand, what we perceive as an advantage of your
patch (ease of use to the user) is perceived as a danger by others: i.e.
it is easy to just click through a dialog and users won't understand the
risk. I'm not convinced that making them jump through unnecessary hoops
like forcing them to add the -shell-escape option themselves will make
them be more careful, and I think Jürgen makes a strong point that
making them add the option to the converter is likely more unsafe
because they will forget to remove it. But since this is a security
issue, let's be careful and make sure others agree.

Helge, are your concerns expressed in [1] and [2] applicable to this
specific patch as well? If they are, it would be great if you could make
an argument for why the potential disadvantages have a greater cost than
the benefits that we claim (ease of use to the user), with regards to
this specific patch. It is a little unfair for me to ask you to make the
argument, since this is a security issue so the burden is actually on us
since we want to change the current behavior. But I think this would be
a good way to start the discussion and we would really appreciate your
opinion on this.

Scott


[1]
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg198111.html
[2]
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg198123.html


signature.asc
Description: PGP signature

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Tue, Jun 13, 2017 at 01:46:45AM -0400, Richard Heck wrote:
> On 06/12/2017 08:08 PM, Enrico Forestieri wrote:
> > On Mon, Jun 12, 2017 at 05:49:50PM -0400, Scott Kostyshak wrote:
> >> On Mon, Jun 12, 2017 at 07:37:53PM +0200, Enrico Forestieri wrote:
> >>
> >>> However, I think I have to wait for a nod before applying this patch.
> >> Thanks for waiting. I think there is indeed a chance Guillaume would be
> >> against it, and if even one person is against it since it is related to
> >> security I think we should have a discussion. I base that guess on [1],
> >> but perhaps I'm wrong. Guillaume?
> > Note that I was convinced by the following observations by Jürgen:
> >
> >> I find it much more dangerous to encourage the user to set the flag
> >> generally, since this might bite him with other documents quite
> >> horribly.
> >> The note in the minted example file advises users
> >> to set the flag. And they would explicitly have to reset it every time.
> >> Chance is high that they just keep it eventually. That's my point.
> > After thinking about it, I agree completely.
> 
> I have not been able to follow all the details of this conversation.
> Since it seems to
> raise VERY important security issues, I wonder if someone could start a
> new thread
> and there summarize the pros and cons of whatever courses of action are
> open to us.
> We are all responsible together for LyX's security, so I'd prefer myself
> if we had a
> wider-ranging discussion of this.
> 
> Generally speaking, we have always been very cautious around these sorts
> of issues.
> See, e.g., the removal of the ability to launch URLs from within LyX.

Richard, this is a different issue. The user can always set -shell-escape
and we can't prevent him from doing that. If the user is bothered from
having to set the option and then delete it again, it may happen that
he leaves it in place, giving rise to all security risks this entails.
We cannot bury our head in the sand and blame the user for that.
Rather, we should try to protect him by setting the option only when
it is really needed, in an ephemeral way. This does not happen behind
the user's back and he is warned of that, hence he can properly act.

-- 
Enrico

Re: [LyX/master] Add accelerator

[Richard Heck]

On 06/12/2017 08:08 PM, Enrico Forestieri wrote:
> On Mon, Jun 12, 2017 at 05:49:50PM -0400, Scott Kostyshak wrote:
>> On Mon, Jun 12, 2017 at 07:37:53PM +0200, Enrico Forestieri wrote:
>>
>>> However, I think I have to wait for a nod before applying this patch.
>> Thanks for waiting. I think there is indeed a chance Guillaume would be
>> against it, and if even one person is against it since it is related to
>> security I think we should have a discussion. I base that guess on [1],
>> but perhaps I'm wrong. Guillaume?
> Note that I was convinced by the following observations by Jürgen:
>
>> I find it much more dangerous to encourage the user to set the flag
>> generally, since this might bite him with other documents quite
>> horribly.
>> The note in the minted example file advises users
>> to set the flag. And they would explicitly have to reset it every time.
>> Chance is high that they just keep it eventually. That's my point.
> After thinking about it, I agree completely.

I have not been able to follow all the details of this conversation.
Since it seems to
raise VERY important security issues, I wonder if someone could start a
new thread
and there summarize the pros and cons of whatever courses of action are
open to us.
We are all responsible together for LyX's security, so I'd prefer myself
if we had a
wider-ranging discussion of this.

Generally speaking, we have always been very cautious around these sorts
of issues.
See, e.g., the removal of the ability to launch URLs from within LyX.

Richard

Re: [LyX/master] Add accelerator

[Jürgen Spitzmüller]

Am Montag, den 12.06.2017, 19:37 +0200 schrieb Enrico Forestieri:
> The attached patch does this. It hooks into the needauth machinery
> and
> everytime any latex converter is called to typeset a minted document,
> the needauth dialogs are used. It is not needed to explicitly set the
> needauth flag and the -shell-escape option is automatically set for
> the
> current latex run. The user can choose to not be bothered by those
> questions for this particular document, so that no question is asked
> afterwards.
> 
> However, I think I have to wait for a nod before applying this patch.

+1 from my (FWIW).

Jürgen

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

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Mon, Jun 12, 2017 at 05:49:50PM -0400, Scott Kostyshak wrote:
> On Mon, Jun 12, 2017 at 07:37:53PM +0200, Enrico Forestieri wrote:
> 
> > However, I think I have to wait for a nod before applying this patch.
> 
> Thanks for waiting. I think there is indeed a chance Guillaume would be
> against it, and if even one person is against it since it is related to
> security I think we should have a discussion. I base that guess on [1],
> but perhaps I'm wrong. Guillaume?

Note that I was convinced by the following observations by Jürgen:

> I find it much more dangerous to encourage the user to set the flag
> generally, since this might bite him with other documents quite
> horribly.

> The note in the minted example file advises users
> to set the flag. And they would explicitly have to reset it every time.
> Chance is high that they just keep it eventually. That's my point.

After thinking about it, I agree completely.

-- 
Enrico

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Mon, Jun 12, 2017 at 05:49:49PM -0400, Scott Kostyshak wrote:
> On Sun, Jun 11, 2017 at 07:50:23PM +0200, Enrico Forestieri wrote:
> 
> > > In that case I think I'd prefer a combo box "Code highlighting package:
> > > Listings|Minted" or somesuch.
> > 
> > I tried to be minimalist in the changes, but if Scott agrees I could also
> > do that. This could delay trasnslations, though, as my spare time is
> > running out.
> 
> Since Uwe is away, I'm guessing he hasn't sent an email to translators
> (hopefully he will respond to [1] when he is back), so I'm fine with it
> if you can finish the patch before I clear up the situation about the
> translators. When do you think you can finish the patch by?

I committed the patch at ae561677. This introduces new strings, so I
am also going to remerge po files (maybe overnight nobody will be upset ;)

-- 
Enrico

Re: [LyX/master] Add accelerator

[Scott Kostyshak]

On Mon, Jun 12, 2017 at 07:37:53PM +0200, Enrico Forestieri wrote:

> However, I think I have to wait for a nod before applying this patch.

Thanks for waiting. I think there is indeed a chance Guillaume would be
against it, and if even one person is against it since it is related to
security I think we should have a discussion. I base that guess on [1],
but perhaps I'm wrong. Guillaume?

I think Guillaume may be away for some days, but beta is still at least
2 weeks away (waiting for a response on [2]), so we will have time to
decide whether we want this patch in beta. And looking at the patch no
strings are changed so we can make a decision post-translations and
pre-beta.

Scott

[1]
https://www.mail-archive.com/search?l=mid=oh9vrf%24h76%241%40blaine.gmane.org
[2]
https://www.mail-archive.com/search?l=mid=20170610172702.j5faogzqg25vu7pu%40steph

Re: [LyX/master] Add accelerator

[Scott Kostyshak]

On Sun, Jun 11, 2017 at 07:50:23PM +0200, Enrico Forestieri wrote:

> > In that case I think I'd prefer a combo box "Code highlighting package:
> > Listings|Minted" or somesuch.
> 
> I tried to be minimalist in the changes, but if Scott agrees I could also
> do that. This could delay trasnslations, though, as my spare time is
> running out.

Since Uwe is away, I'm guessing he hasn't sent an email to translators
(hopefully he will respond to [1] when he is back), so I'm fine with it
if you can finish the patch before I clear up the situation about the
translators. When do you think you can finish the patch by?

Scott

Re: [LyX/master] Add accelerator

[Scott Kostyshak]

On Mon, Jun 12, 2017 at 10:07:55AM +0200, Jürgen Spitzmüller wrote:

> But currently, it isn't. The note in the minted example file advises users
> to set the flag. And they would explicitly have to reset it every time.
> Chance is high that they just keep it eventually. That's my point.

I agree that many users will not unset it.

> I've seen user irritation on similar an much clearer LaTeX messages.

+1

> Many users will not know what " You must invoke LaTeX with the
> -shell-escape flag" means.

Agreed, but I do think that minted users are at least more likely to
know. But I agree we want to make things easier and more safe for the
users.

Scott

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Mon, Jun 12, 2017 at 10:07:55AM +0200, Jürgen Spitzmüller wrote:
> 2017-06-11 19:50 GMT+02:00 Enrico Forestieri :
> > On Sun, Jun 11, 2017 at 07:23:45PM +0200, Jürgen Spitzmüller wrote:
> > >
> > > As I said, I propose that the user has to explicitly acknowledge a
> > > warning message (with "Do not show again for this document" checkbox)
> > > before the document is processed.
> >
> > Remember that we now have the needsauth (or whatever it is called) option
> > now, so that we could also exploit it.
> >
> 
> Yes, right.

The attached patch does this. It hooks into the needauth machinery and
everytime any latex converter is called to typeset a minted document,
the needauth dialogs are used. It is not needed to explicitly set the
needauth flag and the -shell-escape option is automatically set for the
current latex run. The user can choose to not be bothered by those
questions for this particular document, so that no question is asked
afterwards.

However, I think I have to wait for a nod before applying this patch.

-- 
Enrico
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 6e10b18..a84c535 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -279,20 +279,29 @@ OutputParams::FLAVOR 
Converters::getFlavor(Graph::EdgePath const & path,
 }
 
 
-bool Converters::checkAuth(Converter const & conv, string const & doc_fname)
+bool Converters::checkAuth(Converter const & conv, string const & doc_fname,
+  bool use_minted)
 {
-   if (!conv.need_auth())
+   if (!conv.need_auth() && !use_minted)
return true;
-   const docstring security_warning = bformat(
+   string conv_command = conv.command();
+   bool const has_shell_escape = contains(conv_command, "-shell-escape");
+   size_t const token_pos = conv_command.find("$$");
+   bool const has_token = token_pos != string::npos;
+   string const command = conv.latex() && use_minted && !has_shell_escape
+   ? (has_token ? conv_command.insert(token_pos, "-shell-escape ")
+: conv_command.append(" -shell-escape"))
+   : conv_command;
+   docstring const security_warning = bformat(
  _("The requested operation requires the use of a converter 
from "
"%2$s to %3$s:"
"%1$s"
"This external program can execute arbitrary commands on 
your "
"system, including dangerous ones, if instructed to do so by a "
"maliciously crafted .lyx document."),
- from_utf8(conv.command()), from_utf8(conv.from()),
+ from_utf8(command), from_utf8(conv.from()),
  from_utf8(conv.to()));
-   if (lyxrc.use_converter_needauth_forbidden) {
+   if (lyxrc.use_converter_needauth_forbidden && !use_minted) {
frontend::Alert::error(
_("An external converter is disabled for security reasons"),
security_warning + _(
@@ -302,7 +311,7 @@ bool Converters::checkAuth(Converter const & conv, string 
const & doc_fname)
"Forbid needauth converters.)"), false);
return false;
}
-   if (!lyxrc.use_converter_needauth)
+   if (!lyxrc.use_converter_needauth && !use_minted)
return true;
static const docstring security_title =
_("An external converter requires your authorization");
@@ -459,7 +468,8 @@ bool Converters::convert(Buffer const * buffer,
   "tmpfile.out"));
}
 
-   if (!checkAuth(conv, buffer ? buffer->absFileName() : string()))
+   if (!checkAuth(conv, buffer ? buffer->absFileName() : string(),
+  buffer && buffer->params().use_minted))
return false;
 
if (conv.latex()) {
@@ -470,6 +480,10 @@ bool Converters::convert(Buffer const * buffer,
command = subst(command, token_from, "");
command = subst(command, token_latex_encoding,

buffer->params().encoding().latexName());
+   if (buffer->params().use_minted
+   && !contains(command, "-shell-escape"))
+   command += " -shell-escape ";
+
LYXERR(Debug::FILES, "Running " << command);
if (!runLaTeX(*buffer, command, runparams, errorList))
return false;
diff --git a/src/Converter.h b/src/Converter.h
index 1ea7327..297083c 100644
--- a/src/Converter.h
+++ b/src/Converter.h
@@ -194,7 +194,8 @@ public:
/// authorization is: always denied if 
lyxrc.use_converter_needauth_forbidden
/// is enabled; always allowed if the lyxrc.use_converter_needauth
/// is disabled; user is prompted otherwise
-   bool checkAuth(Converter const & 

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Mon, Jun 12, 2017 at 10:07:55AM +0200, Jürgen Spitzmüller wrote:
> 
> I meant: Is the checkbox disabled in context where it does not make sense
> (i.e., for docbook classes?)

No, it isn't, but it is irrelevant in this case. Currently, you can
also enter parameters even if they have no meaning in this context.
The whole Document > Settings > Listings pane should be disabled, instead.

> > It could be a one-shot use. The flag is reset after each latex run.
> >
> 
> But currently, it isn't. The note in the minted example file advises users
> to set the flag. And they would explicitly have to reset it every time.
> Chance is high that they just keep it eventually. That's my point.

However, remember that one can simply apply the changes without
saving them. So, everything is fine when LyX is restarted.

> > I think that minted does a good job here. This is the error you get:
> >
> > ! Package minted Error: You must invoke LaTeX with the -shell-escape flag.
> > See the minted package documentation for explanation.
> > Type  H   for immediate help.
> >
> 
> I've seen user irritation on similar an much clearer LaTeX messages. Many
> users will not know what " You must invoke LaTeX with the -shell-escape
> flag" means.

I see. But those are desperate cases, anyway. I have also seen complaints
about what does it mean opening a terminal for issuing a command. Or even
what issuing a command means.

-- 
Enrico

Re: [LyX/master] Add accelerator

[Jürgen Spitzmüller]

2017-06-11 19:50 GMT+02:00 Enrico Forestieri :

> On Sun, Jun 11, 2017 at 07:23:45PM +0200, Jürgen Spitzmüller wrote:
>
> > Am Sonntag, den 11.06.2017, 18:32 +0200 schrieb Enrico Forestieri:
> > > I don't think that listings is less advanced in code highlighting.
> > > The advantage of minted is that it is less sensitive to the encoding.
> > > All workarounds adopted for listings are not necessary for minted.
> > > I also like better the way a listing is typeset, but this is a
> > > personal preference. So, I would not qualify listings as less
> > > advanced
> > > than minted. However, a label saying "Different code highlighting"
> > > is not any more informative than "Use minted".
> >
> > In that case I think I'd prefer a combo box "Code highlighting package:
> > Listings|Minted" or somesuch.
>
> I tried to be minimalist in the changes, but if Scott agrees I could also
> do that. This could delay trasnslations, though, as my spare time is
> running out.
>

Thanks.


> > "Use minted" is one of these LaTeXisms we want to avoid in the UI.
>
> Maybe that is why pure LaTeX users don't love LyX? ;-)
>

No, that's a more fundamental opposition towards everything that is not a
pure editor.


>
> > What happens if you check "Use minted" outside LaTeX BTW?
>
> I don't understand the question. But if you mean what changes in the UI
> other than the LaTeX output, the answer is that in some dialogs (listings
> and child documents) some options are disabled and in the advanced
> settings tab the recognized options change.
>

I meant: Is the checkbox disabled in context where it does not make sense
(i.e., for docbook classes?)


>
> > > > And LyX should add the -shell-escape flag for
> > > > minted documents (but warn the user before issuing it).
> > >
> > > Hmm... I think you are opening a pandora's box. Maybe we could add
> > > a toolbar button for doing that, so that the user has to
> > > intentionally
> > > do something for getting the -shell-escape flag.
> >
> > As I said, I propose that the user has to explicitly acknowledge a
> > warning message (with "Do not show again for this document" checkbox)
> > before the document is processed.
>
> Remember that we now have the needsauth (or whatever it is called) option
> now, so that we could also exploit it.
>

Yes, right.


>
> > I find it much more dangerous to encourage the user to set the flag
> > generally, since this might bite him with other documents quite
> > horribly.
>
> It could be a one-shot use. The flag is reset after each latex run.
>

But currently, it isn't. The note in the minted example file advises users
to set the flag. And they would explicitly have to reset it every time.
Chance is high that they just keep it eventually. That's my point.



>
> > The problem of both these things is that users get an error message
> > they might not understand if they try minted.
>
> I think that minted does a good job here. This is the error you get:
>
> ! Package minted Error: You must invoke LaTeX with the -shell-escape flag.
> See the minted package documentation for explanation.
> Type  H   for immediate help.
>

I've seen user irritation on similar an much clearer LaTeX messages. Many
users will not know what " You must invoke LaTeX with the -shell-escape
flag" means.

Jürgen


>
> --
> Enrico
>

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Sun, Jun 11, 2017 at 10:57:18PM +0200, Kornel Benko wrote:

> Am Sonntag, 11. Juni 2017 um 22:19:15, schrieb Enrico Forestieri 
> 
> > On Sun, Jun 11, 2017 at 07:23:45PM +0200, Jürgen Spitzmüller wrote:
> > > Am Sonntag, den 11.06.2017, 18:32 +0200 schrieb Enrico Forestieri:
> > > > > I also think this widget should be disabled if not all necessary
> > > > > packages are installed.
> > > > 
> > > > This is problematic, because we should check for installed python
> > > > modules, which I don't think we currently do. 
> > > 
> > > This can be added to configure.py, no?
> > 
> > I see a further difficulty here. The pygments module could be installed
> > as a python 2 or python 3 module. The right python version to use is
> > determined by the used pygmentize command. So, if we are running python 2
> > and don't find the pygments module, it is not an indication that it is
> > not available. Conversely if we are running python 3 and pygments is
> > installed as a python 2 module.
> > 
> > Maybe, the only sensible thing to do is checking for a pygmentize command
> > and, if not found, warn the user but don't disable the widget.
> > 
> 
> Couldn't the test be as easy as to check output of
>   python -c "help('pygments');" 
> ?

Please, read more carefully what written above. In my case,
python -c "help('pygments');"
gives the help, but
python3 -c "help('pygments');"
produces:
No Python documentation found for 'pygments'.
Use help() to get the interactive help utility.
Use help(str) for help on the str class.

Only the driver command knows what python has to be used.

-- 
Enrico

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Sun, Jun 11, 2017 at 10:19:15PM +0200, Enrico Forestieri wrote:
> 
> Maybe, the only sensible thing to do is checking for a pygmentize command
> and, if not found, warn the user but don't disable the widget.

Something like the attached.

-- 
Enrico
diff --git a/lib/configure.py b/lib/configure.py
index 1ce081cd07..8e7fedbb5a 100644
--- a/lib/configure.py
+++ b/lib/configure.py
@@ -1203,6 +1203,8 @@ def checkOtherEntries():
 'splitindex.class'], rc_entry = [ r'\splitindex_command "%%"' ])
 checkProg('a nomenclature processor', ['makeindex'],
 rc_entry = [ r'\nomencl_command "makeindex -s nomencl.ist"' ])
+checkProg('a python-pygments driver command', ['pygmentize'],
+rc_entry = [ r'\pygmentize_command "%%"' ])
 ## FIXME: OCTAVE is not used anywhere
 # path, OCTAVE = checkProg('Octave', ['octave'])
 ## FIXME: MAPLE is not used anywhere
@@ -1756,7 +1758,7 @@ if __name__ == '__main__':
 lyx_check_config = True
 lyx_kpsewhich = True
 outfile = 'lyxrc.defaults'
-lyxrc_fileformat = 21
+lyxrc_fileformat = 22
 rc_entries = ''
 lyx_keep_temps = False
 version_suffix = ''
diff --git a/lib/scripts/prefs2prefs_prefs.py b/lib/scripts/prefs2prefs_prefs.py
index b4acdc74d3..68b4d837dc 100644
--- a/lib/scripts/prefs2prefs_prefs.py
+++ b/lib/scripts/prefs2prefs_prefs.py
@@ -90,6 +90,10 @@
 #   default now)
 #   No conversion necessary.
 
+# Incremented to format 22, by ef
+#   Add pygmentize_command for the python pygments syntax highlighter
+#   No conversion necessary.
+
 # NOTE: The format should also be updated in LYXRC.cpp and
 # in configure.py.
 
@@ -387,5 +391,6 @@ conversions = [
[ 18, []],
[ 19, [remove_print_support]],
[ 20, []],
-   [ 21, []]
+   [ 21, []],
+   [ 22, []]
 ]
diff --git a/src/LyXRC.cpp b/src/LyXRC.cpp
index ae3569096e..e6244f2d2b 100644
--- a/src/LyXRC.cpp
+++ b/src/LyXRC.cpp
@@ -59,7 +59,7 @@ namespace {
 
 // The format should also be updated in configure.py, and conversion code
 // should be added to prefs2prefs_prefs.py.
-static unsigned int const LYXRC_FILEFORMAT = 21; // spitz: jbibtex_alternatives
+static unsigned int const LYXRC_FILEFORMAT = 22; // ef: pygmentize_command
 
 // when adding something to this array keep it sorted!
 LexerKeyword lyxrcTags[] = {
@@ -158,6 +158,7 @@ LexerKeyword lyxrcTags[] = {
{ "\\print_landscape_flag", LyXRC::RC_PRINTLANDSCAPEFLAG },
{ "\\print_paper_dimension_flag", LyXRC::RC_PRINTPAPERDIMENSIONFLAG },
{ "\\print_paper_flag", LyXRC::RC_PRINTPAPERFLAG },
+   { "\\pygmentize_command", LyXRC::RC_PYGMENTIZE_COMMAND },
{ "\\save_compressed", LyXRC::RC_SAVE_COMPRESSED },
{ "\\save_origin", LyXRC::RC_SAVE_ORIGIN },
{ "\\screen_dpi", LyXRC::RC_SCREEN_DPI },
@@ -241,6 +242,7 @@ void LyXRC::setDefaults()
fontenc = "default";
index_command = "makeindex -c -q";
nomencl_command = "makeindex -s nomencl.ist";
+   pygmentize_command = string();
dpi = 75;
// Because a screen is typically wider than a piece of paper:
zoom = 150;
@@ -544,6 +546,12 @@ LyXRC::ReturnValues LyXRC::read(Lexer & lexrc, bool 
check_format)
lexrc >> print_paper_flag;
break;
 
+   case RC_PYGMENTIZE_COMMAND:
+   if (lexrc.next(true)) {
+   pygmentize_command = lexrc.getString();
+   }
+   break;
+
case RC_VIEWDVI_PAPEROPTION:
if (lexrc.next())
view_dvi_paper_option = lexrc.getString();
@@ -1501,6 +1509,13 @@ void LyXRC::write(ostream & os, bool 
ignore_system_lyxrc, string const & name) c
}
if (tag != RC_LAST)
break;
+   case RC_PYGMENTIZE_COMMAND:
+   if (ignore_system_lyxrc ||
+   pygmentize_command != system_lyxrc.pygmentize_command) {
+   os << "\\pygmentize_command \"" << 
escapeCommand(pygmentize_command) << "\"\n";
+   }
+   if (tag != RC_LAST)
+   break;
case RC_TEX_EXPECTS_WINDOWS_PATHS:
// Don't write this setting to the preferences file,
// but allow temporary changes (bug 7557).
@@ -2809,6 +2824,7 @@ void actOnUpdatedPrefs(LyXRC const & lyxrc_orig, LyXRC 
const & lyxrc_new)
case LyXRC::RC_JBIBTEX_ALTERNATIVES:
case LyXRC::RC_JINDEX_COMMAND:
case LyXRC::RC_NOMENCL_COMMAND:
+   case LyXRC::RC_PYGMENTIZE_COMMAND:
case LyXRC::RC_INPUT:
case LyXRC::RC_KBMAP:
case LyXRC::RC_KBMAP_PRIMARY:
@@ -3067,6 +3083,10 @@ string const LyXRC::getDescription(LyXRCTags tag)
str = _("Define the options of makeindex (cf. man makeindex) to 
be used for nomenclatures. This might differ from the index processing 
options.");
   

Re: [LyX/master] Add accelerator

[Kornel Benko]

Am Sonntag, 11. Juni 2017 um 22:19:15, schrieb Enrico Forestieri 

> On Sun, Jun 11, 2017 at 07:23:45PM +0200, Jürgen Spitzmüller wrote:
> > Am Sonntag, den 11.06.2017, 18:32 +0200 schrieb Enrico Forestieri:
> > > > I also think this widget should be disabled if not all necessary
> > > > packages are installed.
> > > 
> > > This is problematic, because we should check for installed python
> > > modules, which I don't think we currently do. 
> > 
> > This can be added to configure.py, no?
> 
> I see a further difficulty here. The pygments module could be installed
> as a python 2 or python 3 module. The right python version to use is
> determined by the used pygmentize command. So, if we are running python 2
> and don't find the pygments module, it is not an indication that it is
> not available. Conversely if we are running python 3 and pygments is
> installed as a python 2 module.
> 
> Maybe, the only sensible thing to do is checking for a pygmentize command
> and, if not found, warn the user but don't disable the widget.
> 

Couldn't the test be as easy as to check output of
python -c "help('pygments');" 
?

Kornel

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

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Sun, Jun 11, 2017 at 07:23:45PM +0200, Jürgen Spitzmüller wrote:
> Am Sonntag, den 11.06.2017, 18:32 +0200 schrieb Enrico Forestieri:
> > > I also think this widget should be disabled if not all necessary
> > > packages are installed.
> > 
> > This is problematic, because we should check for installed python
> > modules, which I don't think we currently do. 
> 
> This can be added to configure.py, no?

I see a further difficulty here. The pygments module could be installed
as a python 2 or python 3 module. The right python version to use is
determined by the used pygmentize command. So, if we are running python 2
and don't find the pygments module, it is not an indication that it is
not available. Conversely if we are running python 3 and pygments is
installed as a python 2 module.

Maybe, the only sensible thing to do is checking for a pygmentize command
and, if not found, warn the user but don't disable the widget.

-- 
Enrico

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Sun, Jun 11, 2017 at 07:23:45PM +0200, Jürgen Spitzmüller wrote:

> Am Sonntag, den 11.06.2017, 18:32 +0200 schrieb Enrico Forestieri:
> > I don't think that listings is less advanced in code highlighting.
> > The advantage of minted is that it is less sensitive to the encoding.
> > All workarounds adopted for listings are not necessary for minted.
> > I also like better the way a listing is typeset, but this is a
> > personal preference. So, I would not qualify listings as less
> > advanced
> > than minted. However, a label saying "Different code highlighting"
> > is not any more informative than "Use minted".
> 
> In that case I think I'd prefer a combo box "Code highlighting package:
> Listings|Minted" or somesuch.

I tried to be minimalist in the changes, but if Scott agrees I could also
do that. This could delay trasnslations, though, as my spare time is
running out.

> "Use minted" is one of these LaTeXisms we want to avoid in the UI.

Maybe that is why pure LaTeX users don't love LyX? ;-)

> What happens if you check "Use minted" outside LaTeX BTW?

I don't understand the question. But if you mean what changes in the UI
other than the LaTeX output, the answer is that in some dialogs (listings
and child documents) some options are disabled and in the advanced
settings tab the recognized options change.

> > > And LyX should add the -shell-escape flag for
> > > minted documents (but warn the user before issuing it).
> > 
> > Hmm... I think you are opening a pandora's box. Maybe we could add
> > a toolbar button for doing that, so that the user has to
> > intentionally
> > do something for getting the -shell-escape flag.
> 
> As I said, I propose that the user has to explicitly acknowledge a
> warning message (with "Do not show again for this document" checkbox)
> before the document is processed.

Remember that we now have the needsauth (or whatever it is called) option
now, so that we could also exploit it.

> I find it much more dangerous to encourage the user to set the flag
> generally, since this might bite him with other documents quite
> horribly.

It could be a one-shot use. The flag is reset after each latex run.

> The problem of both these things is that users get an error message
> they might not understand if they try minted.

I think that minted does a good job here. This is the error you get:

! Package minted Error: You must invoke LaTeX with the -shell-escape flag.
See the minted package documentation for explanation.
Type  H   for immediate help.

-- 
Enrico

Re: [LyX/master] Add accelerator

[Guillaume MM]

Le 11/06/2017 à 16:23, Jürgen Spitzmüller a écrit :

And LyX should add the -shell-escape flag for
minted documents (but warn the user before issuing it).




Hi Jürgen, this is being discussed in this thread:

https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg200515.html

Re: [LyX/master] Add accelerator

[Jürgen Spitzmüller]

Am Sonntag, den 11.06.2017, 18:32 +0200 schrieb Enrico Forestieri:
> I don't think that listings is less advanced in code highlighting.
> The advantage of minted is that it is less sensitive to the encoding.
> All workarounds adopted for listings are not necessary for minted.
> I also like better the way a listing is typeset, but this is a
> personal preference. So, I would not qualify listings as less
> advanced
> than minted. However, a label saying "Different code highlighting"
> is not any more informative than "Use minted".

In that case I think I'd prefer a combo box "Code highlighting package:
Listings|Minted" or somesuch. "Use minted" is one of these LaTeXisms we
want to avoid in the UI.

What happens if you check "Use minted" outside LaTeX BTW?

> > I also think this widget should be disabled if not all necessary
> > packages are installed.
> 
> This is problematic, because we should check for installed python
> modules, which I don't think we currently do. 

This can be added to configure.py, no?

> We could check whether
> the pygmentize command is available, but we cannot be sure it is
> called exactly that way. Indeed, minted provides a command for
> specifying it. You can do \renewcommand{\MintedPygmentize}{pygments}
> if the command to be called is pygments instead of pygmentize,
> for example. So we risk of gratuitously disabling a functionality.
> 
> > And LyX should add the -shell-escape flag for
> > minted documents (but warn the user before issuing it).
> 
> Hmm... I think you are opening a pandora's box. Maybe we could add
> a toolbar button for doing that, so that the user has to
> intentionally
> do something for getting the -shell-escape flag.

As I said, I propose that the user has to explicitly acknowledge a
warning message (with "Do not show again for this document" checkbox)
before the document is processed.

I find it much more dangerous to encourage the user to set the flag
generally, since this might bite him with other documents quite
horribly.


The problem of both these things is that users get an error message
they might not understand if they try minted.

Jürgen

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

Re: [LyX/master] Add accelerator

[Enrico Forestieri]

On Sun, Jun 11, 2017 at 04:23:45PM +0200, Jürgen Spitzmüller wrote:

> Am Sonntag, den 11.06.2017, 13:58 +0200 schrieb Enrico Forestieri:
> > -  Use minted
> > +  Use minted
> 
> Is there a label that is less opaque to people who do not know what
> "minted" is? Something that highlights the benefits one gets when
> clicking this widget? E.g., "Advanced code highlighting" (and then
> explain it uses the minted package in the tool tip).

I don't think that listings is less advanced in code highlighting.
The advantage of minted is that it is less sensitive to the encoding.
All workarounds adopted for listings are not necessary for minted.
I also like better the way a listing is typeset, but this is a
personal preference. So, I would not qualify listings as less advanced
than minted. However, a label saying "Different code highlighting"
is not any more informative than "Use minted".

> I also think this widget should be disabled if not all necessary
> packages are installed.

This is problematic, because we should check for installed python
modules, which I don't think we currently do. We could check whether
the pygmentize command is available, but we cannot be sure it is
called exactly that way. Indeed, minted provides a command for
specifying it. You can do \renewcommand{\MintedPygmentize}{pygments}
if the command to be called is pygments instead of pygmentize,
for example. So we risk of gratuitously disabling a functionality.

> And LyX should add the -shell-escape flag for
> minted documents (but warn the user before issuing it).

Hmm... I think you are opening a pandora's box. Maybe we could add
a toolbar button for doing that, so that the user has to intentionally
do something for getting the -shell-escape flag.

-- 
Enrico

Re: [LyX/master] Add accelerator

[Jürgen Spitzmüller]

Am Sonntag, den 11.06.2017, 13:58 +0200 schrieb Enrico Forestieri:
> -  Use minted
> +  Use minted

Is there a label that is less opaque to people who do not know what
"minted" is? Something that highlights the benefits one gets when
clicking this widget? E.g., "Advanced code highlighting" (and then
explain it uses the minted package in the tool tip).

I also think this widget should be disabled if not all necessary
packages are installed. And LyX should add the -shell-escape flag for
minted documents (but warn the user before issuing it).

Jürgen

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