Re: Japanese docs: incorrect paths?

[Uwe Stöhr]

El 25.10.2017 a las 19:53, Scott Kostyshak escribió:


Uwe, do you happen to have time to take a look at what is going wrong?


I fixes them now - I hope.
Koji, there are now 2 untranslated image captions in sec. 4.6.2.


Also, why don't missing images give an error when compiling? This way
our ctests would have caught this problem.


The problem is that I cannot see these things because I cannot compile 
Japanese documents.


regards Uwe

Re: Update on the 2.3.0rc1 situation

[Richard Heck]

On 10/24/2017 01:29 AM, Scott Kostyshak wrote:
> On Mon, Oct 23, 2017 at 08:44:07AM +, Enrico Forestieri wrote:
>> On Sun, Oct 22, 2017 at 09:36:56PM -0400, Richard Heck wrote:
>>> On 10/22/2017 06:19 PM, Scott Kostyshak wrote:
 On Sat, Oct 14, 2017 at 06:37:06AM +, Jürgen Spitzmüller wrote:

> Also, I think we should consider Günter's lyx2lyx patch [1], but I
> didn't have time to thoroughly review it myself.
 Will anyone have time to take a look at the patch by Wednesday?
>>> I could look at the code from a code-triaging point of view, but I have
>>> not followed
>>> the controversy about this, so someone would need to tell me what the
>>> code is supposed
>>> to do.
>> I don't think this is a change that should be performed at a RC1 stage,
>> frankly. It is too risky and of dubious utility.
> Thanks for bringing up this concern. The patch is not trivial. Further,
> the patch only deals with backwards compatibility. From what I recall,
> we place a higher importance on forward conversion, and although we do
> make efforts to provide good backward compatibility, I believe that we
> have at times knowingly not implemented certain functionality in our
> backwards conversion. Indeed, our "Development" manual covers this:
>
>   While the conversion routine is required to produce a document that
>   is equivalent to the old version, the requirements of the reversion
>   are not that strict. If possible, try to produce a proper reversion,
>   using ERT if needed, but for some features this might be too
>   complicated. In this case, the minimum requirement of the reversion
>   routine is that it produces a valid document which can be read by an
>   older LyX. If absolutely needed, even data loss is allowed for the
>   reversion.
>
> The current code (without the patch) clearly already satisfies the
> "minimum requirement". All this to say: I don't think the patch is too
> important for 2.3.0 and in my opinion I'm fine if we do not put it in.
>
> Since I don't think the patch is critical, and since we are hopefully a
> couple of days from going forward with a freeze for RC1, I propose that
> we should only consider this patch if a developer gives a "very
> confident" +1, and if we have an extensive test suite. 

The whole lyx2lyx test suite issue is one we should sort out, but we don't
have any such thing at this point.

I don't have a strong opinion on the issues that led to this patch, but
it seems to me that we aredoing a lot here, too much for this late in the
game. The actual problem is the deletion of ZWSP characters, which 72a488d7
said was needed "so that they don't accumulate". The concern here, I take
it, is that reverting to a pre-2.3.x format and then re-importing might
lead to a stack of these things. Why not just make sure that doesn't
happen? I.e., check if there's a stack of them, and if so fix that?

Richard

Re: Initial view of document (master)

[Richard Heck]

On 10/25/2017 02:35 PM, Kornel Benko wrote:
> Am Dienstag, 24. Oktober 2017 um 19:51:41, schrieb Jean-Marc Lasgouttes 
> 
>> Le 23/10/2017 à 22:26, Pavel Sanda a écrit :
>>> And I have reproducible crash:
>>> 1. start new document
>>> 2. write "ambititious ", spellcheck correctly underlies text.
>>> 3. try to fix spelling via context menu, choose "ambitious"
>>> 4. kaboom
>> I can't reproduce. Qt4 or Qt5?
>>
>> JMarc
> I can reproduce with continuous spellcheck enabled. Qt5.
>
> It crashes on LASSERT(LASSERT(end > start && end <= size() + 1, return false);
>  at src/Paragraph.cpp:612
> (gdb) p start
> $1 = 0
> (gdb) p end
> $2 = 11
> (gdb) p size()
> $1 = 9
>
> Without this assert, everything works OK. Richard, why is it there needed?

"git blame" will blame me, but I only added the "return false". That
way, in release mode, we just say "no change" and continue, hopefully
safely. The assertion itself goes back to d53d4a5c, in 2006.

Presumably, the end variable does not really make sense here. Position
11 is past the end of the paragraph (and past the end of paragraph
marker at pos=10). We're lucky, because Changes::isChanged isn't
affected by this. Something is definitely wrong, though, and that's what
the assertion is telling us.

Richard




Richard

Re: Update on the 2.3.0rc1 situation

[Scott Kostyshak]

On Mon, Oct 23, 2017 at 08:44:07AM +, Enrico Forestieri wrote:
> On Sun, Oct 22, 2017 at 09:36:56PM -0400, Richard Heck wrote:
> > On 10/22/2017 06:19 PM, Scott Kostyshak wrote:
> > > On Sat, Oct 14, 2017 at 06:37:06AM +, Jürgen Spitzmüller wrote:
> > >
> > >> Also, I think we should consider Günter's lyx2lyx patch [1], but I
> > >> didn't have time to thoroughly review it myself.
> > > Will anyone have time to take a look at the patch by Wednesday?
> > 
> > I could look at the code from a code-triaging point of view, but I have
> > not followed
> > the controversy about this, so someone would need to tell me what the
> > code is supposed
> > to do.
> 
> I don't think this is a change that should be performed at a RC1 stage,
> frankly. It is too risky and of dubious utility.

Thanks for bringing up this concern. The patch is not trivial. Further,
the patch only deals with backwards compatibility. From what I recall,
we place a higher importance on forward conversion, and although we do
make efforts to provide good backward compatibility, I believe that we
have at times knowingly not implemented certain functionality in our
backwards conversion. Indeed, our "Development" manual covers this:

  While the conversion routine is required to produce a document that
  is equivalent to the old version, the requirements of the reversion
  are not that strict. If possible, try to produce a proper reversion,
  using ERT if needed, but for some features this might be too
  complicated. In this case, the minimum requirement of the reversion
  routine is that it produces a valid document which can be read by an
  older LyX. If absolutely needed, even data loss is allowed for the
  reversion.

The current code (without the patch) clearly already satisfies the
"minimum requirement". All this to say: I don't think the patch is too
important for 2.3.0 and in my opinion I'm fine if we do not put it in.

Since I don't think the patch is critical, and since we are hopefully a
couple of days from going forward with a freeze for RC1, I propose that
we should only consider this patch if a developer gives a "very
confident" +1, and if we have an extensive test suite. Although an
extensive test suite cannot catch all potential issues, it can certainly
help us be more confident for a patch like this.

I cannot confidently review the patch, but if another developer
carefully reviews the patch and gives a confident +1, and if Günter has
time to help, I can create a test suite of many different scenarios.
This would consist of different .lyx files created by current LyX 2.3.x,
and we would check the export to 2.2.x with and without the patch. We
could also check the loading of the resulting files in e.g. LyX 2.2.x,
and the LaTeX output from 2.2.x.

There are a lot of "if's" above and we have a short amount of time, so
I'm not sure it's worth it at this point. But I will leave it to others
to decide. If another developer thinks it is worth it enough to spend
time carefully reviewing the patch, then I volunteer to spend time
working on the test suite.

Scott


signature.asc
Description: PGP signature

Re: Initial view of document (master)

[Kornel Benko]

Am Dienstag, 24. Oktober 2017 um 19:51:41, schrieb Jean-Marc Lasgouttes 

> Le 23/10/2017 à 22:26, Pavel Sanda a écrit :
> > And I have reproducible crash:
> > 1. start new document
> > 2. write "ambititious ", spellcheck correctly underlies text.
> > 3. try to fix spelling via context menu, choose "ambitious"
> > 4. kaboom
> 
> I can't reproduce. Qt4 or Qt5?
> 
> JMarc

I can reproduce with continuous spellcheck enabled. Qt5.

It crashes on LASSERT(LASSERT(end > start && end <= size() + 1, return false);
 at src/Paragraph.cpp:612
(gdb) p start
$1 = 0
(gdb) p end
$2 = 11
(gdb) p size()
$1 = 9


Without this assert, everything works OK. Richard, why is it there needed?

Kornel

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

Re: Freeze 2.3.x on Wednesday for rc1?

[Scott Kostyshak]

On Tue, Oct 24, 2017 at 03:13:38PM +, Guenter Milde wrote:
> On 2017-10-22, Uwe Stöhr wrote:
> > El 22.10.2017 a las 20:08, Scott Kostyshak escribió:
> 
> >> I would like to propose to freeze the 2.3.x branch at 23:59 UTC on
> >> Wednesday, for releasing rc1.
> 
> > +1
> 
> > I just checked the docs that all changes are now accepted.
> 
> There is still the pending patch with the dash documentation update.

From what I understand, no one has objected to the documentation patch,
except for the part to RELEASE-NOTES (which I believe we have resolved).
Thus, unless I misunderstood something, please go ahead and commit to
2.3.x.

Thanks, Günter.

Scott


signature.asc
Description: PGP signature

Re: Show changes don't work properly with display math

[Enrico Forestieri]

On Wed, Oct 25, 2017 at 12:55:34PM -0400, Scott Kostyshak wrote:
> On Mon, Oct 23, 2017 at 05:32:59PM +, Enrico Forestieri wrote:
> 
> > Scott, what about 2.3.x?
> 
> Please go ahead. Thanks,

Thanks, done.

-- 
Enrico

Test

[Richard Heck]

Low volume today, so

Japanese docs: incorrect paths?

[Scott Kostyshak]

Open ja/Embedded.lyx. Scroll down a bit and you will see that some
images are not found. For example, 2D-intensity-plot.pdf. The path for
this one is:

/clipart/2D-intensity-plot.pdf

From what I understand, it should be:

../clipart/2D-intensity-plot.pdf

right? Or am I misunderstanding something?

I think there are many cases.

Note that not all images should be prepended with "..". It depends on
whether the image is localized (there is a "clipart" folder inside the
folder "ja").

Also, why don't missing images give an error when compiling? This way
our ctests would have caught this problem.

Uwe, do you happen to have time to take a look at what is going wrong?

Thanks,

Scott


signature.asc
Description: PGP signature

Re: Update on the 2.3.0rc1 situation

[Guenter Milde]

On 2017-10-23, Richard Heck wrote:
> On 10/22/2017 06:19 PM, Scott Kostyshak wrote:
>> On Sat, Oct 14, 2017 at 06:37:06AM +, Jürgen Spitzmüller wrote:

>>> Also, I think we should consider Günter's lyx2lyx patch [1], but I
>>> didn't have time to thoroughly review it myself.
>> Will anyone have time to take a look at the patch by Wednesday?

> I could look at the code from a code-triaging point of view, 

That would be nice.

> but I have not followed the controversy about this, so someone would
> need to tell me what the code is supposed to do.

The patch cures the following problems in the current 2.3 lyx2lyx code:

a) "\use_dash_ligatures" is set based on the original file version,
   regardless of content.
   If you used literal em- and en-dashes in pre-2.2 documents, you must
   manually unselect "Output em- and en-dash as ligatures" to ensure
   unchanged behaviour.

b) The value of the "\use_dash_ligatures" setting is lost after a round-trip
   conversion:
   2.3 -> 2.2 -> 2.3 forces "\use_dash_ligatures false" and 
   2.3 -> 2.1 -> 2.3 forces "\use_dash_ligatures true".

c) Conversion 2.3 -> 2.2 adds a literal ZWSP character after dashes, if
   \use_dash_ligatures is True. Conversion 2.2 -> 2.3 removes any ZWSP
   following a dash.

   * ZWSP characters (u200b) following literal em- and en-dashes are deleted by
 lyx2lyx when converting to 2.3 format. If you used them as optional line
 breaks after dashes, convert them to space insets before opening your
 document with LyX 2.3 or the optional line breaks will be lost!
   
   * If using TeX fonts and en- and em-dashes are output as font ligatures,
 when exporting documents containing en- and em-dashes to the format of
 LyX 2.0 or earlier, the following line has to be manually added to the
 unicodesymbols file of that LyX version:
 0x200b "\\hspace{0pt}" "" "" "" "" # ZERO WIDTH SPACE
 This avoids "uncodable character" issues if the document is actually
 loaded by that LyX version. LyX 2.1 and later versions already have the
 necessary definition in their unicodesymbols file.

d) Conversion 2.3 -> 2.1 (or older) uses literal dashes also if
   \use_dash_ligatures is True -- although LyX<=2.1 supports ligature
   dashes.


a) and b) are fixed by a content based setting:

Motivation: incompatible changes to older document occur due to different
line-break handling for literal vs. ligature dashes. 

1. scan the document for literal dashes followed by a word or no-break space

2. set \use_dash_ligatures for the 4 cases:

   literal dashes  ligature dashes   \use_dash_ligatures
   
   yes nofalse  
   no  yes   true
   no  nodefault¹
   yes yes   default¹
   
   ¹ don't set \use_dash_ligatures in lyx2lyx
 --> the default is added by LyX.


c) and d) are fixed by a change to the workaround for LyX 2.2 to use
   preamble code instead of "invisible" ZWSP characters in the body. The
   preamble code is removed when converting from 2.2 (both directions).

The changes solve the issues without adding complexity:

  3 files changed, 87 insertions(+), 104 deletions(-)

Günter

Re: Show changes don't work with URL (and maybe other insets)

[racoon]

On 20.10.2017 09:46, racoon wrote:
Open and typeset the attached file with a deleted URL and active show 
changes. It shows both the actual and expected result.


Notes: While the current algorithm seems fine for some insets, like 
footnotes, it falters at other insets, like URL, which do not interpret 
its content as LaTeX code.


Okay, it seems pretty impossible to solve this. The problem is that when 
only plain text is allowed in an inset, like URL, changes can be marked 
as on whole inset but not different changes within it.


However, here is what one might reasonably do:

if the content is only plain, i.e. forceplain is true, to not end the 
\lyxadded and \lyxdeleted before it but just wrap it around the inset.


and additionally, but I guess this is not such a simple change,

have an inset setting that disables change tracking within it, so that 
there will be no \lyxadded and \lyxdeleted within the inset.


Daniel

Re: Freeze 2.3.x on Wednesday for rc1?

[Guenter Milde]

On 2017-10-22, Uwe Stöhr wrote:
> El 22.10.2017 a las 20:08, Scott Kostyshak escribió:

>> I would like to propose to freeze the 2.3.x branch at 23:59 UTC on
>> Wednesday, for releasing rc1.

> +1

> I just checked the docs that all changes are now accepted.

There is still the pending patch with the dash documentation update.

Günter


Exec: git 'diff' 'UserGuide.lyx' 2>&1
Dir: /usr/local/src/lyx/lib/doc/

diff --git a/lib/doc/UserGuide.lyx b/lib/doc/UserGuide.lyx
index 188a16dc6f..e19bd5a5d8 100644
--- a/lib/doc/UserGuide.lyx
+++ b/lib/doc/UserGuide.lyx
@@ -140,11 +140,12 @@ enumitem
 \papercolumns 1
 \papersides 2
 \paperpagestyle default
-\tracking_changes false
+\tracking_changes true
 \output_changes false
 \html_math_output 0
 \html_css_as_file 0
 \html_be_strict true
+\author -1402925745 "Günter Milde"
 \end_header
 
 \begin_body
@@ -16114,6 +16115,8 @@ minus sign
 \end_layout
 
 \begin_layout Standard
+
+\change_deleted -1402925745 1508170801
 \begin_inset Box Frameless
 position "t"
 hor_pos "c"
@@ -16402,6 +16405,279 @@ minus sign
 \end_inset
 
 
+\change_inserted -1402925745 1508170773
+
+\begin_inset Tabular
+
+
+
+
+
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+name
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+output
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+inserted with
+\end_layout
+
+\end_inset
+
+
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+hyphen
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+-
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+\begin_inset Quotes eld
+\end_inset
+
+
+\family typewriter
+-
+\family default
+
+\begin_inset Quotes erd
+\end_inset
+
+ in text
+\end_layout
+
+\end_inset
+
+
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+en dash
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+–
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+\begin_inset CommandInset href
+LatexCommand href
+name "system key combination"
+target 
"https://en.wikipedia.org/wiki/Dash#Encoding.2C_substitution.2C_and_keyboard_shortcuts;
+literal "false"
+
+\end_inset
+
+
+\begin_inset Note Note
+status collapsed
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+note to translators: Please change the link to point to a version in your
+ language (see left side-bar in the wikipedia page).
+\end_layout
+
+\end_inset
+
+ or 
+\begin_inset Quotes eld
+\end_inset
+
+--
+\begin_inset Quotes erd
+\end_inset
+
+ in text
+\end_layout
+
+\end_inset
+
+
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+em dash
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+—
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+\begin_inset CommandInset href
+LatexCommand href
+name "system key combination"
+target 
"https://en.wikipedia.org/wiki/Dash#Encoding.2C_substitution.2C_and_keyboard_shortcuts;
+literal "false"
+
+\end_inset
+
+
+\begin_inset Note Note
+status collapsed
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+note to translators: Please change the link to point to a version in your
+ language (see left side-bar in the wikipedia page).
+\end_layout
+
+\end_inset
+
+ or 
+\begin_inset Quotes eld
+\end_inset
+
+---
+\begin_inset Quotes erd
+\end_inset
+
+ in text
+\end_layout
+
+\end_inset
+
+
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+minus sign
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+\begin_inset Formula $-$
+\end_inset
+
+
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted -1402925745 1508170773
+\begin_inset Quotes eld
+\end_inset
+
+
+\family typewriter
+-
+\family default
+
+\begin_inset Quotes erd
+\end_inset
+
+ in math mode
+\end_layout
+
+\end_inset
+
+
+
+
+\end_inset
+
+
+\change_unchanged
+
 \begin_inset VSpace defskip
 \end_inset
 
@@ -16428,7 +16704,40 @@ unicode-insert
 \end_inset
 
  \SpecialChar LyX
- function.
+ function
+\change_inserted -1402925745 1508169009
+ (with argument 2013 for the em dash and 2014 for the en dash)
+\change_unchanged
+.
+\change_inserted -1402925745 1508169032
+
+\begin_inset Foot
+status collapsed
+
+\begin_layout Plain Layout
+

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Le 23/10/2017 ?? 22:21, Pavel Sanda a écrit :
>> Another cursor issue (I have vague memory this was already discussed):
>> while editing paragraph of text go to the end of last word.
>> Hit spacebar - space is inserted, but cursor stays at the end of word
>> and shows only after inserting the next non whitespace character.
>
> This one? http://www.lyx.org/trac/ticket/10117

Yep. Pavel

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
> Le 23/10/2017 ?? 22:26, Pavel Sanda a écrit :
>> And I have reproducible crash:
>> 1. start new document
>> 2. write "ambititious ", spellcheck correctly underlies text.
>> 3. try to fix spelling via context menu, choose "ambitious"
>> 4. kaboom
>
> I can't reproduce. Qt4 or Qt5?

IIRC I got it home with qt4, I just got assertion on my work box with qt5 as 
well:
lassert.cpp (51): ASSERTION end > start && end <= size() + 1 VIOLATED IN 
Paragraph.cpp:612

Pavel

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 23/10/2017 à 22:26, Pavel Sanda a écrit :

And I have reproducible crash:
1. start new document
2. write "ambititious ", spellcheck correctly underlies text.
3. try to fix spelling via context menu, choose "ambitious"
4. kaboom


I can't reproduce. Qt4 or Qt5?

JMarc

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

Le 23/10/2017 à 22:21, Pavel Sanda a écrit :

Another cursor issue (I have vague memory this was already discussed):
while editing paragraph of text go to the end of last word.
Hit spacebar - space is inserted, but cursor stays at the end of word
and shows only after inserting the next non whitespace character.


This one? http://www.lyx.org/trac/ticket/10117

JMarc

Re: PATCH for 2.3, use frescobaldi as preferred editor for lilypond material

[Scott Kostyshak]

On Sat, Oct 14, 2017 at 02:40:32AM +, Scott Kostyshak wrote:
> On Fri, Oct 13, 2017 at 06:34:46PM +, Richard Heck wrote:
> 
> > It can certainly go to master. I don't see why we shouldn't do it for
> > 2.3.x myself, so I'd give it the necessary +1, but I guess I feel like
> > Scott should probably chime in on this.
> 
> Thanks, yes, for feature patches I like to be involved.
> 
> This feature is fine with me, so since Richard +1'd it, go ahead for
> 2.3.0. Thanks for the patch, Helge.

Helge or Richard, please commit as soon as possible if you would like
this in 2.3.0rc1.

Also, please note my comments below.

> I have two comments:
> 
> 1. Note that there are three different case styles: "Lilypond files",
> "lilypond editor", "LilyPond music". Perhaps that's intentional (e.g.
> "LyX software" vs. the "lyx binary") but I wanted to point it out.
> 
> 2. Can we avoid the duplication of the list of text editors? I'm
> guessing there is no simple way to avoid re-searching for duplicate
> binaries {i.e. just using the result of checkViewerEditor('a text
> editor'} but can we at least create a variable called "text_editors", or
> something like that, which we can reuse? It seems that our search for a
> BiBTeX editor also copies the list of text editors.
> 
> (not related to patch, just a random thought) Regarding (2) above, a
> more ambitious (not for 2.3.x) approach might be to use a text editor as
> the backup editor for any format that has a MIME "text/whatever". It
> doesn't seem that currently would make a difference for many cases
> though, so probably not worth the effort.
> 
> By the way, the Frescobaldi looks really cool and is under active
> development so that's good to see.
> 
> Scott

Scott


signature.asc
Description: PGP signature

Re: [PATCH] to avoid crash in misspelled marker painting while deleting table rows fast

[Scott Kostyshak]

On Fri, Oct 13, 2017 at 05:24:50AM +, Stephan Witt wrote:
> Am 11.10.2017 um 12:10 schrieb Jean-Marc Lasgouttes :
> > 
> > Le 08/10/2017 à 18:01, Stephan Witt a écrit :
> >> Hi,
> >> these days I’ve got a crash while deleting multiple rows from a table with 
> >> bottom table toolbar quickly.
> >> This happened with LyX 2.2. - I can reproduce it with master.
> >> I’m attaching the crash report and a patch to avoid the crash. I’d like to 
> >> put it in and if possible backport it to 2.3.x at least. Ok?
> > 
> > A question: is the 'quickly' important to get the crash? From what I see, 
> > Cursor::newWord() is not updated to the changes in the document structure. 
> > Why does this happen?
> 
> Sorry for the delay. I tried to reproduce and cannot anymore ATM. I had the 
> impression 'quickly‘ was important to get the crash. It didn’t happen on the 
> first or second deletion.
> 
> I don’t understand your statement regarding the change in document structure 
> and the following question.

What shall we do with this regarding rc1? If for some reason we do not
put in Stephan's patch, can we at least put in some debug code or an
assert or something to help us make progress on this in the future?

Scott


signature.asc
Description: PGP signature

Re: Update on the 2.3.0rc1 situation

[Scott Kostyshak]

On Mon, Oct 23, 2017 at 08:44:07AM +, Enrico Forestieri wrote:
> On Sun, Oct 22, 2017 at 09:36:56PM -0400, Richard Heck wrote:
> > On 10/22/2017 06:19 PM, Scott Kostyshak wrote:
> > > On Sat, Oct 14, 2017 at 06:37:06AM +, Jürgen Spitzmüller wrote:
> > >
> > >> Also, I think we should consider Günter's lyx2lyx patch [1], but I
> > >> didn't have time to thoroughly review it myself.
> > > Will anyone have time to take a look at the patch by Wednesday?
> > 
> > I could look at the code from a code-triaging point of view, but I have
> > not followed
> > the controversy about this, so someone would need to tell me what the
> > code is supposed
> > to do.
> 
> I don't think this is a change that should be performed at a RC1 stage,
> frankly. It is too risky and of dubious utility.

Thanks for bringing up this concern. The patch is not trivial. Further,
the patch only deals with backwards compatibility. From what I recall,
we place a higher importance on forward conversion, and although we do
make efforts to provide good backward compatibility, I believe that we
have at times knowingly not implemented certain functionality in our
backwards conversion. Indeed, our "Development" manual covers this:

  While the conversion routine is required to produce a document that
  is equivalent to the old version, the requirements of the reversion
  are not that strict. If possible, try to produce a proper reversion,
  using ERT if needed, but for some features this might be too
  complicated. In this case, the minimum requirement of the reversion
  routine is that it produces a valid document which can be read by an
  older LyX. If absolutely needed, even data loss is allowed for the
  reversion.

The current code (without the patch) clearly already satisfies the
"minimum requirement". All this to say: I don't think the patch is too
important for 2.3.0 and in my opinion I'm fine if we do not put it in.

Since I don't think the patch is critical, and since we are so close to
RC1, I propose that we should only consider this patch if a developer
gives a confident +1, and if we have an extensive test suite. Although
an extensive test suite cannot catch all potential issues, it can
certainly help us be more confident for a patch like this.

I cannot confidently review the patch, but if another developer
carefully reviews the patch and gives a confident +1, and if Günter has
time to help, I can create a test suite of many different scenarios.
This would consist of different .lyx files created by current LyX 2.3.x,
and we would check the export to 2.2.x with and without the patch. We
could also check the loading of the resulting files in e.g. LyX 2.2.x,
and the LaTeX output from 2.2.x.

There are a lot of "if's" above and we have a short amount of time, so
I'm not sure it's worth it at this point. But I will leave it to others
to decide. If another developer thinks it is worth it enough to spend
time carefully reviewing the patch, then I volunteer to spend time
working on the test suite.

Scott


signature.asc
Description: PGP signature

Re: Show changes don't work properly with display math

[Scott Kostyshak]

On Mon, Oct 23, 2017 at 05:32:59PM +, Enrico Forestieri wrote:

> Scott, what about 2.3.x?

Please go ahead. Thanks,

Scott


signature.asc
Description: PGP signature

Re: Initial view of document (master)

[Pavel Sanda]

Jean-Marc Lasgouttes wrote:
>> After some testing:
>> - initial thread report present
>
> Which one?

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

>> - a is present
>
> I think it is gone now

Confirmed, this issue is gone.

>> - c mostly fixed, ghost c3*** still remains

See the attached file. Put caret into note.
Left arrow //you get out of note
Left arrow //you get in front of 'f', this is place where ghost remains
Left arrow //caret is correctly moved but ghost remains after 'f'

Can you see it now?

Pavel


pok.lyx
Description: application/lyx

Re: Show changes don't work properly with display math

[Scott Kostyshak]

On Mon, Oct 23, 2017 at 05:32:59PM +, Enrico Forestieri wrote:

> Scott, what about 2.3.x?

Please go ahead. Thanks,

Scott


signature.asc
Description: PGP signature

Re: Initial view of document (master)

[Jean-Marc Lasgouttes]

I added an early return for Update::None that got forgotten.

Le 20/10/2017 à 16:56, Pavel Sanda a écrit :

After some testing:
- initial thread report present


Which one?


- a is present


I think it is gone now


- b is fixed
- c mostly fixed, ghost c3*** still remains


Cannot reproduce, but I am not sure of what it is.


Given that none of the remaining is critical I'd say go and commit,
I'll start new threads for new issues.


It is in now.

JMarc

Re: LyX-Workarea: Background not shown correctly

[Jean-Marc Lasgouttes]

Le 23/10/2017 à 23:34, Patrick De Visschere a écrit :

With the patch in place, SingleParUpdate in bufferView::draw is never reached.


So finally you mean that an UpdateRequest is really fired for each 
update event (which makes sense :), right?



It would be interesting to set a break on QWindow::requestUpdate and see 
whether to is something we do that triggers this effect.


I did and it’s never triggered.


OK.


If I pass the coordinates of the paragraph, I notice that
“SingleParUpdate" actually means “SingleRowUpdate". Since only the
current paragraph is painted black now, except for the current row.


What happens is that the painting code (in TextMetrics::drawParagraph) 
only paints the rows of the paragraph that have changed. This is why you 
see that only the row cursor is repainted. If you do a change that 
changes the cursor row and the next one (by inserting a character), you 
should see several rows repainted.


I do not like much the idea of just restricting the update area, since 
it seems very fragile. But we can return to it if needed. There are ways 
to get all the coordinates that we need.


There is a point that I would like to clear though: is the screen turned 
to black at each event (insertion of a character...), or only when 
certain things happen, like the example of doing a PDF preview like 
Stephan described at the beginning of this thread?



I’m still having problems with the basics. I thought there existed a document 
describing the basics of textMetrics, paragraphMetrics … .
I found something on the wiki but not so much.


Did you take a look at development/PAINTING_ANALYSIS? I try to maintain 
it while doing changes, but it is incomplete, and probably wrong in some 
places.


JMarc

Re: LyX-Workarea: Background not shown correctly

[Patrick De Visschere]

> On 25 Oct 2017, at 12:21, Jean-Marc Lasgouttes  wrote:
> 
> Le 23/10/2017 à 23:34, Patrick De Visschere a écrit :
>> With the patch in place, SingleParUpdate in bufferView::draw is never 
>> reached.
> 
> So finally you mean that an UpdateRequest is really fired for each update 
> event (which makes sense :), right?

The patch turns every update into a FullScreenUpdate and SingleParUpdate is 
then obsolete.
Every call to viewport()->update() leads to a QEvent::updateRequest I guess.

> 
>>> It would be interesting to set a break on QWindow::requestUpdate and see 
>>> whether to is something we do that triggers this effect.
>> I did and it’s never triggered.
> 
> OK.

btw this was about the function requestUpdate(), not the QEvent.

> 
 If I pass the coordinates of the paragraph, I notice that
 “SingleParUpdate" actually means “SingleRowUpdate". Since only the
 current paragraph is painted black now, except for the current row.
> 
> What happens is that the painting code (in TextMetrics::drawParagraph) only 
> paints the rows of the paragraph that have changed. This is why you see that 
> only the row cursor is repainted. If you do a change that changes the cursor 
> row and the next one (by inserting a character), you should see several rows 
> repainted.

Yes I found the crcCheck on a row.
But when forcing a FullScreenUpdate this code becomes useless, since with 
pi.full_repaint=true every line is redrawn.

> 
> I do not like much the idea of just restricting the update area, since it 
> seems very fragile. But we can return to it if needed. There are ways to get 
> all the coordinates that we need.

I agree. The dimensions passed via update(x,y,w,h) and the actual area painted 
must match exactly. I cannot judge the penalty of the screen redraws. I doesn’t 
seem to hurt very much.
Nevertheless I have now something which works mostly. There are still problems 
when dragging a selection but I believe these can be solved by fine tuning. The 
other problem is that when opening a document the screen stil turns black.

> 
> There is a point that I would like to clear though: is the screen turned to 
> black at each event (insertion of a character...), or only when certain 
> things happen, like the example of doing a PDF preview like Stephan described 
> at the beginning of this thread?

When typing, the whole screen stays black, except for the current line, which 
looks normal. You don’t have to do anything special to get this.
That’s what I expect: when typing a single character in a row, only that row 
will be repainted but the update() triggers a full screen erase.

With a big document it’s sufficient to scroll a little bit to trigger a 
FullScreenUpdate. So maybe when not typing and moving the mouse around it might 
not always be obvious.
And within a table there is no problem, meaning that the cells do not turn 
black. Moving the mouse over a table seems to trigger a FullScreenUpdate.

> 
>> I’m still having problems with the basics. I thought there existed a 
>> document describing the basics of textMetrics, paragraphMetrics … .
>> I found something on the wiki but not so much.
> 
> Did you take a look at development/PAINTING_ANALYSIS? I try to maintain it 
> while doing changes, but it is incomplete, and probably wrong in some places.

Yes I found that and it helped.

> 
> JMarc




smime.p7s
Description: S/MIME cryptographic signature

Re: LyX-Workarea: Background not shown correctly

[Patrick De Visschere]

> On 25 Oct 2017, at 16:28, Jean-Marc Lasgouttes  wrote:
> 
> Le 25/10/2017 à 16:15, Patrick De Visschere a écrit :
>>> So finally you mean that an UpdateRequest is really fired for each update 
>>> event (which makes sense :), right?
>> The patch turns every update into a FullScreenUpdate and SingleParUpdate is 
>> then obsolete.
>> Every call to viewport()->update() leads to a QEvent::updateRequest I guess.
> 
> Yes, I meant without the patch that does a full repaint on each full paint 
> event.
> 
>> btw this was about the function requestUpdate(), not the QEvent.
> 
> I suspect they are the same, but the Qt source is really too big for me. I 
> tried to understand what happens, but it is hard to grasp.
> 
>> Yes I found the crcCheck on a row.
>> But when forcing a FullScreenUpdate this code becomes useless, since with 
>> pi.full_repaint=true every line is redrawn.
> 
> Of course, if we force, we force.
> 
>>> I do not like much the idea of just restricting the update area, since it 
>>> seems very fragile. But we can return to it if needed. There are ways to 
>>> get all the coordinates that we need.
>> I agree. The dimensions passed via update(x,y,w,h) and the actual area 
>> painted must match exactly. I cannot judge the penalty of the screen 
>> redraws. I doesn’t seem to hurt very much.
>> Nevertheless I have now something which works mostly. There are still 
>> problems when dragging a selection but I believe these can be solved by fine 
>> tuning. The other problem is that when opening a document the screen stil 
>> turns black.
> 
> And the other problem is that we do a lot of useless painting (that is 
> ignored by the clipping but probably still costs us).
> 
>>> There is a point that I would like to clear though: is the screen turned to 
>>> black at each event (insertion of a character...), or only when certain 
>>> things happen, like the example of doing a PDF preview like Stephan 
>>> described at the beginning of this thread?
>> When typing, the whole screen stays black, except for the current line, 
>> which looks normal. You don’t have to do anything special to get this.
>> That’s what I expect: when typing a single character in a row, only that row 
>> will be repainted but the update() triggers a full screen erase.
>> With a big document it’s sufficient to scroll a little bit to trigger a 
>> FullScreenUpdate. So maybe when not typing and moving the mouse around it 
>> might not always be obvious.
>> And within a table there is no problem, meaning that the cells do not turn 
>> black. Moving the mouse over a table seems to trigger a FullScreenUpdate.
> 
> OK, assume that you managed to get afull repaint (with some scrolling for 
> example). Now, if you type a character, does it turn the whole work area to 
> black?
> 

Yes (except for some insets, as always) and when in a table the whole table 
remains normal.

> JMarc




smime.p7s
Description: S/MIME cryptographic signature

Re: LyX-Workarea: Background not shown correctly

[Jean-Marc Lasgouttes]

Le 25/10/2017 à 16:15, Patrick De Visschere a écrit :

So finally you mean that an UpdateRequest is really fired for each update event 
(which makes sense :), right?


The patch turns every update into a FullScreenUpdate and SingleParUpdate is 
then obsolete.
Every call to viewport()->update() leads to a QEvent::updateRequest I guess.


Yes, I meant without the patch that does a full repaint on each full 
paint event.



btw this was about the function requestUpdate(), not the QEvent.


I suspect they are the same, but the Qt source is really too big for me. 
I tried to understand what happens, but it is hard to grasp.



Yes I found the crcCheck on a row.
But when forcing a FullScreenUpdate this code becomes useless, since with 
pi.full_repaint=true every line is redrawn.


Of course, if we force, we force.


I do not like much the idea of just restricting the update area, since it seems 
very fragile. But we can return to it if needed. There are ways to get all the 
coordinates that we need.


I agree. The dimensions passed via update(x,y,w,h) and the actual area painted 
must match exactly. I cannot judge the penalty of the screen redraws. I doesn’t 
seem to hurt very much.
Nevertheless I have now something which works mostly. There are still problems 
when dragging a selection but I believe these can be solved by fine tuning. The 
other problem is that when opening a document the screen stil turns black.


And the other problem is that we do a lot of useless painting (that is 
ignored by the clipping but probably still costs us).



There is a point that I would like to clear though: is the screen turned to 
black at each event (insertion of a character...), or only when certain things 
happen, like the example of doing a PDF preview like Stephan described at the 
beginning of this thread?


When typing, the whole screen stays black, except for the current line, which 
looks normal. You don’t have to do anything special to get this.
That’s what I expect: when typing a single character in a row, only that row 
will be repainted but the update() triggers a full screen erase.

With a big document it’s sufficient to scroll a little bit to trigger a 
FullScreenUpdate. So maybe when not typing and moving the mouse around it might 
not always be obvious.
And within a table there is no problem, meaning that the cells do not turn 
black. Moving the mouse over a table seems to trigger a FullScreenUpdate.


OK, assume that you managed to get afull repaint (with some scrolling 
for example). Now, if you type a character, does it turn the whole work 
area to black?


JMarc