Re: Only show Accept/Reject Change options if relevant
On 05/05/2018 03:53 PM, Scott Kostyshak wrote: > On Sat, May 05, 2018 at 05:11:31PM +, Scott Kostyshak wrote: >> On Sat, May 05, 2018 at 04:05:49PM +, Richard Kimberly Heck wrote: >>> On 05/05/2018 11:56 AM, Scott Kostyshak wrote: The attached patch is an attempt to only show the "Accept Change" and "Reject Change" options in the context menu when there is a change in the selection. It works well except if the selection is inside an inset. For example, when in a LyX Note, ppos_beg and ppos_end refer to the position within the note. Does each inset have its own ParagraphList? If so, how do I access it? >>> If it's an InsetText, then it has its own Text object, which has paragraphs: >>> ParagraphList const & pars = theInset.text().paragraphs(); >> Thanks, Riki. that's exactly what I needed. New patch attached. >> >> The next problem is that I need to check all paragraphs on all depths. >> For example, if there is a change inside a LyX Note, and I select the >> LyX Note from the outside of the inset, isChanged() is false because I >> guess it checks whether the entire note was changed, not the contents of >> the note. >> >> I'll look into this. > I'm unsure what approach to take. One option would be to define > "isChanged()" to return true if there is a change in the paragraph or in > any paragraph within the paragraph. I think that currently, isChanged() > checks only the top-level paragraph. I have no idea if this is a > reasonable change to make. You could just define a new method that does this and leave the old one as is. > If that is not a correct approach, then I suppose that I need to check > each paragraph on all levels myself. In this case, is there a way to > iterate through all paragraphs and nested paragraphs of a selection? If > there is not currently a way, would this be useful as a new helper > method? There must be a good way to do this, but I do not know what it is. Riki
Encoding issues when exporting R code from a .lyx file
The attached .lyx file contains a knitr chunk. If I export to PDF, the
PDF is created successfully. However, if I export the .R file and run
source('encoding_export_issue.R', echo = TRUE)
I get an error:
Error in nchar(dep, "c") : invalid multibyte string, element 1
The reason for this error, from the R perspective, is detailed by this
r-help ML post [1]. The problem is that when LyX exports the .Rnw file
(which in turn is used to make the .R file), the .Rnw file is created
with encoding "iso-8859-1". I think that this encoding is consistent
with the encoding that we set for creating .tex files that we export (if
they contain e.g. an "á" character, as is contained in the attached .lyx
file). However, R does not interpret files in the same way as LaTeX
engines do. It seems that LaTeX engines automatically handle .tex files
of various encodings. R, by default, assumes a certain encoding (which
encoding it assumes depends on the output of Sys.getlocale()). For my
system, it appears that UTF-8 is assumed.
I know very little about file encodings. Is there an improvement that we
could make? For example, in the attached .lyx file, would it be
reasonable to save the .Rnw file (which is what is used to create the .R
file) with the default encoding of the system (which in my case seems to
be UTF-8)?
Scott
[1]
https://www.mail-archive.com/search?l=mid&q=20180504204700.o7swd4rmc4uebuot%40Opti1604
encoding_export_issue.lyx
Description: application/lyx
signature.asc
Description: PGP signature
Re: Only show Accept/Reject Change options if relevant
On Sat, May 05, 2018 at 05:11:31PM +, Scott Kostyshak wrote: > On Sat, May 05, 2018 at 04:05:49PM +, Richard Kimberly Heck wrote: > > On 05/05/2018 11:56 AM, Scott Kostyshak wrote: > > > The attached patch is an attempt to only show the "Accept Change" and > > > "Reject Change" options in the context menu when there is a change in > > > the selection. > > > > > > It works well except if the selection is inside an inset. For example, > > > when in a LyX Note, ppos_beg and ppos_end refer to the position within > > > the note. Does each inset have its own ParagraphList? If so, how do I > > > access it? > > > > If it's an InsetText, then it has its own Text object, which has paragraphs: > > ParagraphList const & pars = theInset.text().paragraphs(); > > Thanks, Riki. that's exactly what I needed. New patch attached. > > The next problem is that I need to check all paragraphs on all depths. > For example, if there is a change inside a LyX Note, and I select the > LyX Note from the outside of the inset, isChanged() is false because I > guess it checks whether the entire note was changed, not the contents of > the note. > > I'll look into this. I'm unsure what approach to take. One option would be to define "isChanged()" to return true if there is a change in the paragraph or in any paragraph within the paragraph. I think that currently, isChanged() checks only the top-level paragraph. I have no idea if this is a reasonable change to make. If that is not a correct approach, then I suppose that I need to check each paragraph on all levels myself. In this case, is there a way to iterate through all paragraphs and nested paragraphs of a selection? If there is not currently a way, would this be useful as a new helper method? Scott signature.asc Description: PGP signature
Re: [LyX/master] Towards a sane character dialog
On Sat, May 5, 2018 at 10:51 AM, Jürgen Spitzmüller wrote: > Am Samstag, den 05.05.2018, 12:03 -0400 schrieb Scott Kostyshak: > > Makes sense. In this case, what would you think of putting the text > > "Language" above the combo box? I think this would look nicer and I > > think that it would also reduce the minimum horizontal size of the > > dialog. > > I tried this and committed. Please check it out. > I like this in concert with the other changes. Thanks for your work on this! - Joel
Re: [LyX/master] Towards a sane character dialog
On Sat, May 05, 2018 at 04:51:40PM +, Jürgen Spitzmüller wrote: > Am Samstag, den 05.05.2018, 12:03 -0400 schrieb Scott Kostyshak: > > Makes sense. In this case, what would you think of putting the text > > "Language" above the combo box? I think this would look nicer and I > > think that it would also reduce the minimum horizontal size of the > > dialog. > > I tried this and committed. Please check it out. Thanks, I think it looks good! Scott signature.asc Description: PGP signature
Re: Only show Accept/Reject Change options if relevant
On Sat, May 05, 2018 at 04:05:49PM +, Richard Kimberly Heck wrote:
> On 05/05/2018 11:56 AM, Scott Kostyshak wrote:
> > The attached patch is an attempt to only show the "Accept Change" and
> > "Reject Change" options in the context menu when there is a change in
> > the selection.
> >
> > It works well except if the selection is inside an inset. For example,
> > when in a LyX Note, ppos_beg and ppos_end refer to the position within
> > the note. Does each inset have its own ParagraphList? If so, how do I
> > access it?
>
> If it's an InsetText, then it has its own Text object, which has paragraphs:
> ParagraphList const & pars = theInset.text().paragraphs();
Thanks, Riki. that's exactly what I needed. New patch attached.
The next problem is that I need to check all paragraphs on all depths.
For example, if there is a change inside a LyX Note, and I select the
LyX Note from the outside of the inset, isChanged() is false because I
guess it checks whether the entire note was changed, not the contents of
the note.
I'll look into this.
Scott
From 335f461c26d7aa64c01f7e1f9722d4007e983ce2 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak
Date: Fri, 4 May 2018 18:21:54 -0400
Subject: [PATCH] Only show Accept/Reject Change options if relevant
In the context menu for a selection, we now only show the options
"Accept Change" and "Reject Change" if there are actually changes in
the selection.
---
src/Text3.cpp | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/src/Text3.cpp b/src/Text3.cpp
index 9b0f50c..f0d488b 100644
--- a/src/Text3.cpp
+++ b/src/Text3.cpp
@@ -3185,17 +3185,32 @@ bool Text::getStatus(Cursor & cur, FuncRequest const & cmd,
case LFUN_CHANGE_ACCEPT:
case LFUN_CHANGE_REJECT:
- // In principle, these LFUNs should only be enabled if there
- // is a change at the current position/in the current selection.
- // However, without proper optimizations, this will inevitably
- // result in unacceptable performance - just imagine a user who
- // wants to select the complete content of a long document.
if (!cur.selection())
enable = cur.paragraph().isChanged(cur.pos());
- else
- // TODO: context-sensitive enabling of LFUN_CHANGE_ACCEPT/REJECT
- // for selections.
- enable = true;
+ else {
+ // enable if there is a change in the selection
+ enable = false;
+ ParagraphList const & pars = cur.text()->paragraphs();
+ pit_type const pars_begin = cur.selectionBegin().pit();
+ pit_type const pars_end = cur.selectionEnd().pit();
+ for (pit_type pit = pars_begin; pit <= pars_end; ++pit) {
+pos_type ppos_beg, ppos_end;
+if (pit == pars_begin)
+ ppos_beg = cur.selBegin().pos();
+else
+ ppos_beg = 0;
+if (pit == pars_end)
+ ppos_end = cur.selEnd().pos();
+else
+ ppos_end = pars[pit].size();
+
+if (ppos_beg != ppos_end &&
+pars[pit].isChanged(ppos_beg, ppos_end)) {
+ enable = true;
+ break;
+}
+ }
+ }
break;
case LFUN_OUTLINE_UP:
--
2.7.4
signature.asc
Description: PGP signature
Re: [LyX/master] Towards a sane character dialog
Am Samstag, den 05.05.2018, 12:03 -0400 schrieb Scott Kostyshak: > Makes sense. In this case, what would you think of putting the text > "Language" above the combo box? I think this would look nicer and I > think that it would also reduce the minimum horizontal size of the > dialog. I tried this and committed. Please check it out. Jürgen > > Scott signature.asc Description: This is a digitally signed message part
Re: Only show Accept/Reject Change options if relevant
On 05/05/2018 11:56 AM, Scott Kostyshak wrote: > The attached patch is an attempt to only show the "Accept Change" and > "Reject Change" options in the context menu when there is a change in > the selection. > > It works well except if the selection is inside an inset. For example, > when in a LyX Note, ppos_beg and ppos_end refer to the position within > the note. Does each inset have its own ParagraphList? If so, how do I > access it? If it's an InsetText, then it has its own Text object, which has paragraphs: ParagraphList const & pars = theInset.text().paragraphs(); Riki
Re: [LyX/master] Towards a sane character dialog
On Sat, May 05, 2018 at 03:30:52PM +, Jürgen Spitzmüller wrote: > Am Samstag, den 05.05.2018, 11:18 -0400 schrieb Scott Kostyshak: > > I've been testing and I like it. Thanks for your work on it. I did > > just > > get the following message on the terminal (not sure exactly what I > > did): > > > > frontends/qt4/ButtonPolicy.cpp (228): > > OkApplyCancelAutoReadOnlyPolicy: > > No transition for input SMI_APPLY from state AUTOAPPLY_INITIAL > > > > This was on 39596ab5. > > I have just fixed this. > > > It might also look nicer to vertically align "Color:" with "Family:". > > Yes. Tested and looks good. > > Perhaps also top-align "Semantic Markup" with "Language:" ? > > I've deliberately aligned the widgets, not the group header. Makes sense. In this case, what would you think of putting the text "Language" above the combo box? I think this would look nicer and I think that it would also reduce the minimum horizontal size of the dialog. Scott signature.asc Description: PGP signature
Only show Accept/Reject Change options if relevant
The attached patch is an attempt to only show the "Accept Change" and
"Reject Change" options in the context menu when there is a change in
the selection.
It works well except if the selection is inside an inset. For example,
when in a LyX Note, ppos_beg and ppos_end refer to the position within
the note. Does each inset have its own ParagraphList? If so, how do I
access it?
In addition to the above problem, any other comments are welcome, since
I don't know this code well.
Scott
From 22ea8d15753e15c124f73b929348da42daee3431 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak
Date: Fri, 4 May 2018 18:21:54 -0400
Subject: [PATCH] Only show Accept/Reject Change options if relevant
In the context menu for a selection, we now only show the options
"Accept Change" and "Reject Change" if there are actually changes in
the selection.
---
src/Text3.cpp | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/src/Text3.cpp b/src/Text3.cpp
index 572e4bd..8c2bd00 100644
--- a/src/Text3.cpp
+++ b/src/Text3.cpp
@@ -3191,17 +3191,32 @@ bool Text::getStatus(Cursor & cur, FuncRequest const & cmd,
case LFUN_CHANGE_ACCEPT:
case LFUN_CHANGE_REJECT:
- // In principle, these LFUNs should only be enabled if there
- // is a change at the current position/in the current selection.
- // However, without proper optimizations, this will inevitably
- // result in unacceptable performance - just imagine a user who
- // wants to select the complete content of a long document.
if (!cur.selection())
enable = cur.paragraph().isChanged(cur.pos());
- else
- // TODO: context-sensitive enabling of LFUN_CHANGE_ACCEPT/REJECT
- // for selections.
- enable = true;
+ else {
+ // enable if there is a change in the selection
+ enable = false;
+ ParagraphList & pars = cur.buffer()->text().paragraphs();
+ pit_type const pars_begin = cur.selectionBegin().bottom().pit();
+ pit_type const pars_end = cur.selectionEnd().bottom().pit();
+ for (pit_type pit = pars_begin; pit <= pars_end; ++pit) {
+pos_type ppos_beg, ppos_end;
+if (pit == pars_begin)
+ ppos_beg = cur.selBegin().pos();
+else
+ ppos_beg = 0;
+if (pit == pars_end)
+ ppos_end = cur.selEnd().pos();
+else
+ ppos_end = pars[pit].size();
+
+if (ppos_beg != ppos_end &&
+pars[pit].isChanged(ppos_beg, ppos_end)) {
+ enable = true;
+ break;
+}
+ }
+ }
break;
case LFUN_OUTLINE_UP:
--
2.7.4
signature.asc
Description: PGP signature
Re: [LyX/master] Towards a sane character dialog
On 05/05/2018 03:53 AM, Jürgen Spitzmüller wrote: > Am Freitag, den 04.05.2018, 15:08 -0400 schrieb Richard Kimberly Heck: > >> I think "None" would be better than "Without", too. > I borrowed "(Without)" including the parens from LibreOffice Wrtier's > character dialog. I thought that "None" vs. "No change" is rather > confusing. OK. Riki
Re: [LyX/master] Towards a sane character dialog
Am Samstag, den 05.05.2018, 11:18 -0400 schrieb Scott Kostyshak: > I've been testing and I like it. Thanks for your work on it. I did > just > get the following message on the terminal (not sure exactly what I > did): > > frontends/qt4/ButtonPolicy.cpp (228): > OkApplyCancelAutoReadOnlyPolicy: > No transition for input SMI_APPLY from state AUTOAPPLY_INITIAL > > This was on 39596ab5. I have just fixed this. > It might also look nicer to vertically align "Color:" with "Family:". Yes. > Perhaps also top-align "Semantic Markup" with "Language:" ? I've deliberately aligned the widgets, not the group header. Jürgen > I'm not too > confident though in my UI suggestions. > > Scott signature.asc Description: This is a digitally signed message part
Re: [LyX/master] Towards a sane character dialog
On Fri, May 04, 2018 at 05:39:50PM +, Juergen Spitzmueller wrote: > commit fb393b450d661e8b374a7db0177bad9170043dae > Author: Juergen Spitzmueller > Date: Fri May 4 19:34:09 2018 +0200 > > Towards a sane character dialog > > This is a proposal, but I think you should try it out in order to comment > > What this does, is: > > 1. Remove the toggle madness. This is really not something anyone > understands without knowing the code, and its very unusual UI > (fixes #4836) > > 2. Separate and group things that were put all into the "Misc" trashcan > combo > > 3. Let the dialog reflect the font settings at cursor (selection) > > Now the dialog looks more like character dialogs from other applications, > and I think it is more in line with what users expect. > > Comments very welcome (and of course I will revert if you want to have > the old idiosyncratic thing back). > --- I've been testing and I like it. Thanks for your work on it. I did just get the following message on the terminal (not sure exactly what I did): frontends/qt4/ButtonPolicy.cpp (228): OkApplyCancelAutoReadOnlyPolicy: No transition for input SMI_APPLY from state AUTOAPPLY_INITIAL This was on 39596ab5. It might also look nicer to vertically align "Color:" with "Family:". Perhaps also top-align "Semantic Markup" with "Language:" ? I'm not too confident though in my UI suggestions. Scott signature.asc Description: PGP signature
Re: [LyX/master] Towards a sane character dialog
Am Samstag, 5. Mai 2018 09:53:30 CEST schrieb Jürgen Spitzmüller : > Am Freitag, den 04.05.2018, 15:08 -0400 schrieb Richard Kimberly Heck: > > On 05/04/2018 02:53 PM, Kornel Benko wrote: > > > Am Freitag, 4. Mai 2018 19:39:50 CEST schrieb Juergen Spitzmueller > > > : > > > > commit fb393b450d661e8b374a7db0177bad9170043dae > > > > Author: Juergen Spitzmueller > > > > Date: Fri May 4 19:34:09 2018 +0200 > > > > > > > > Towards a sane character dialog > > > > > > > > > > May we please get a context to the translation of > > > Single,Double[[line spacing]] # Paragraph settings > > > Single,Double[[underline]]# Text style->Customized > > OK. Thanks. > > > > > > Gender problems > > > > I think "None" would be better than "Without", too. > > I borrowed "(Without)" including the parens from LibreOffice Wrtier's > character dialog. I thought that "None" vs. "No change" is rather > confusing. > > Jürgen > > > > > Riki Kornel > signature.asc Description: This is a digitally signed message part.
Jenkins build is back to normal : Build branch "master" » ubuntu-xenial-qt4-autotools-extended #939
https://ci.inria.fr/lyx/job/build-master-head/job/ubuntu-xenial-qt4-autotools-extended/939/
Jenkins build is back to normal : Build branch "master" » ubuntu-latest-qt5-cmake #916
https://ci.inria.fr/lyx/job/build-master-head/job/ubuntu-latest-qt5-cmake/916/
Re: [LyX/master] Towards a sane character dialog
Am Freitag, den 04.05.2018, 15:56 -0400 schrieb Richard Kimberly Heck:
> Now that "toggle all" does not come from the dialog, should we get
> rid
> of that entirely?
> It never made a lot of sense to me. It mostly seems to affect
> LFUN_TEXTSTYLE_APPLY
> at this point.
Probably. I haven't followed that path.
> Also, parts of this code from GuiView.cpp:
>
> case LFUN_DIALOG_SHOW: {
> string const name = cmd.getArg(0);
> string sdata =
> trim(to_utf8(cmd.argument()).substr(name.size()));
>
> if (name == "character") {
> sdata = freefont2string();
> if (!sdata.empty())
> showDialog("character", sdata);
> }
>
> now seem unnecessary. What this used to do was pass information about
> the last used font
> to the dialog. But we now don't use that, do we?
Yes, looks like we didn't, and we don't.
I'll remove that.
Jürgen
>
> Riki
>
signature.asc
Description: This is a digitally signed message part
Re: [LyX/master] Add missing connection after fb393b45.
Am Freitag, den 04.05.2018, 21:12 +0200 schrieb Richard Kimberly Heck: > commit 7f4f14786d480fc46d4250cdf80c2e6a61a46c5c > Author: Richard Kimberly Heck > Date: Fri May 4 15:11:53 2018 -0400 > > Add missing connection after fb393b45. Actually, this was intended (but I forgot to remove the emphCB connection. These two have their dedicated slot (on_emphCB_clicked() and on_nounCB_cklicked() that call change_adaptor at the end (after doing other things). Jürgen > --- > src/frontends/qt4/GuiCharacter.cpp |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/src/frontends/qt4/GuiCharacter.cpp > b/src/frontends/qt4/GuiCharacter.cpp > index 2c3023a..3037d85 100644 > --- a/src/frontends/qt4/GuiCharacter.cpp > +++ b/src/frontends/qt4/GuiCharacter.cpp > @@ -220,6 +220,7 @@ GuiCharacter::GuiCharacter(GuiView & lv) > connect(ulineCO, SIGNAL(activated(int)), this, > SLOT(change_adaptor())); > connect(strikeCO, SIGNAL(activated(int)), this, > SLOT(change_adaptor())); > connect(emphCB, SIGNAL(clicked(bool)), this, > SLOT(change_adaptor())); > + connect(nounCB, SIGNAL(clicked(bool)), this, > SLOT(change_adaptor())); > connect(sizeCO, SIGNAL(activated(int)), this, > SLOT(change_adaptor())); > connect(familyCO, SIGNAL(activated(int)), this, > SLOT(change_adaptor())); > connect(seriesCO, SIGNAL(activated(int)), this, > SLOT(change_adaptor())); signature.asc Description: This is a digitally signed message part
Re: [LyX/master] Towards a sane character dialog
Am Freitag, den 04.05.2018, 15:08 -0400 schrieb Richard Kimberly Heck: > On 05/04/2018 02:53 PM, Kornel Benko wrote: > > Am Freitag, 4. Mai 2018 19:39:50 CEST schrieb Juergen Spitzmueller > > : > > > commit fb393b450d661e8b374a7db0177bad9170043dae > > > Author: Juergen Spitzmueller > > > Date: Fri May 4 19:34:09 2018 +0200 > > > > > > Towards a sane character dialog > > > > > > > May we please get a context to the translation of > > Single,Double[[line spacing]] # Paragraph settings > > Single,Double[[underline]] # Text style->Customized OK. > > > > Gender problems > > I think "None" would be better than "Without", too. I borrowed "(Without)" including the parens from LibreOffice Wrtier's character dialog. I thought that "None" vs. "No change" is rather confusing. Jürgen > > Riki > signature.asc Description: This is a digitally signed message part
