Re: LyX 2.0 compilation errors using CMAKE

[Abdelrazak Younes]

On 09/01/2010 01:38 AM, Uwe Stöhr wrote:

Am 31.08.2010 08:02, schrieb Stephan Witt:


I guess it's my change r35193.

Please try attached patch.


This fixes the setenv compilation errors - I committed it.

However, this compilation error remains:

10..\..\src\LyX.cpp(288) : error C4101: 'message' : unreferenced 
local variable


The attached patch fixes also this error for me (debug and release mode).


I am not sure about lyx_exit() at this stage of the boot process maybe 
just exist() will do. Also AFAIR Alert:: in LyX requires GUI mode so 
make sure your patch works with command-line mode (lyx -e).



It seems that MSVC doesn't allow to use empty catched exceptions.


No, this is a warning about unused variabled but CMake treats these as 
error (on purpose). This is to avoid to let unused variables everywhere 
like it used to be.



OK to put it in?


No, for now just comment out message:

-   } catch (ExceptionMessage const  message) {}
+   } catch (ExceptionMessage const  /*message*/) {


Abdel.

Re: LyX 2.0 compilation errors using CMAKE

[Abdelrazak Younes]

On 08/31/2010 03:15 AM, Uwe Stöhr wrote:

Am 31.08.2010 03:04, schrieb Uwe Stöhr:

I am not able to compile LyX 2.0 using CMAKE. I get these errors:


I forgot to say that I'm using
- Microsoft Visual C++ 2008 Express Edition with SP1
- Qt 4.6.3
- Windows XP 64bit (a.k.a. Windows 2003 Server)
- CMake 2.8.2

and build with this batch file:

-
:: set here the path to Qt's bin folder and to the LyX-MSVC depend bin 
folder

:: (these are example paths)
PATH=D:\Qt\bin;D:\LyXSVN\lyx-devel\lyx-windows-deps-msvc2008\bin;%PATH%

:: change directory to the folder where the compile result should be 
stored

:: (this is an example path)
cd D:\LyXSVN\lyx-devel\compile-result
cmake ..\development\cmake -GVisual Studio 9 2008 -Dnls=1 
-DGNUWIN32_DIR=D:\LyXSVN\lyx-devel\lyx-windows-deps-msvc2008 -Dmerge=0


:: clean
start lyx.sln :: /clean Release

:: rebuild all generated files
::cmake ..\cmake -Dmerge_rebuild=1


First -Dmerge=0 and then -Dmerge_rebuild=1? This seems fishy...

In general when you are developping for debug, disable merge build. For 
release mode I agree that this is faster.


Abdel.

Re: lilybook module

[Guenter Milde]

On 2010-08-31, Julien Rioux wrote:
 On 31/08/2010 3:35 PM, Guenter Milde wrote:
 On 2010-08-30, Jean-Marc Lasgouttes wrote:

 Using the OutputFormat tag defined for use with sweave, one can have a
 module that changes the format (and thus add an extra convert step).
...
 Could this be used to allow LilyPond source code in LyX that is
 converted to musical scores by the 'lilypond-book' pre-processor?

 Yes, I have already submitted a patch to this list to implement it.

Once again, there is a thread on the user list requesting this feature,
so I hope it can go e.g. to the Wiki.

 Since then I have worked on the patch a little bit. It works well except 
 for one or two things. As mentioned in a previous message in this tread, 
 instant preview always treats the current buffer as LaTeX, so any 
 attempt to change the OutputFormat to allow preprocessing by 
 lilypond-book is ignored by the preview mechanism. My current fix to get 
 Instant preview working is kind of a hack, so I haven't posted a updated 
 patch yet.

For me, it would be OK to typeset the LilyPond code verbatim in the
instant preview with a macro redefinition in the preamble that gets
removed/replaced by the pre-processor. I agree this is not WYSIWYG but an
instant fallback solution.

Günter

Re: LyX 2.0 compilation errors using CMAKE

[Peter Kümmel]

On 01.09.2010 01:38, Uwe Stöhr wrote:
 Am 31.08.2010 08:02, schrieb Stephan Witt:
 
 I guess it's my change r35193.

 Please try attached patch.
 
 This fixes the setenv compilation errors - I committed it.
 
 However, this compilation error remains:
 
 10..\..\src\LyX.cpp(288) : error C4101: 'message' : unreferenced local 
 variable

using msvc this warning as handled as error.
but it's fixed now.

 
 The attached patch fixes also this error for me (debug and release mode). It 
 seems that MSVC doesn't 
 allow to use empty catched exceptions. OK to put it in?
 
 thanks and regards
 Uwe

Re: LyX 2.0 compilation errors using CMAKE

[Peter Kümmel]

 First -Dmerge=0 and then -Dmerge_rebuild=1? This seems fishy...
 
 In general when you are developping for debug, disable merge build. For 
 release mode I agree that this is faster.
 
 Abdel.
 

these options are obsolete now. all options now start with LYX_ see cmake output
or cmake-gui.

I've also update development/cmake/build.bat. Hope the output is self 
explaining:

-
Usage build.bat devel/install/deploy STUDIO(optional)
devel   - Builds Visual Studio project files for development on LyX
install - Builds Visual Studio project files with all enabled for 
installation
deploy  - Builds Makefiles and installs LyX
STUDIO  - Used Visual Studio version, default is Visual Studio 9 2008
  use Visual Studio 10 for Visual Studio 10
-
Be sure you've set qmake in PATH and set the variables:
GNUWIN32_DIR
LYX_SOURCE
LXY_BUILD
Or edit this file.
-
ERROR: no options.
Exiting now.


Peter

Re: Patch for spell checker state in Paragraph instead of Font (Re: alpha6)

[Jean-Marc LASGOUTTES]

Stephan Witt st.w...@gmx.net writes:
 I've made some changes again... shorten lines as Jürgen suggested and
 formatting of if/while. And I added code to handle the soft-hyphens
 and friends.

 So I attach it again and hope I can put it in later. JMarc, can you
 have a look, please?

See below. Nice work.

JMarc

 +int AppleSpellChecker::numMisspelledWords() const
 +{
 + return numMisspelledWordsAppleSpeller(d-speller);
 +}

It might be nice, short of having proper objc++ files, to rename the
objc helper functions to something more obvious like
AppleSpeller_numMisspelledWords (or even AS_numMisspelledWords).

 --- src/Text2.cpp (Revision 35252)
 +++ src/Text2.cpp (Arbeitskopie)
 @@ -356,6 +356,9 @@
   Font f = tm.displayFont(pit, pos);
   f.update(font, language, toggleall);
   setCharFont(pit, pos, f, tm.font_);
 + // font change may change language... 
 + // spell checker has to know that
 + pars_[pit].requestSpellCheck();
   }
  }
  


 - void setMisspelled(bool misspelled) { misspelled_ = misspelled; }
 - ///
 - bool isMisspelled() const { return misspelled_; }
 - ///

Yay!


 Index: src/Paragraph.cpp
 ===
 --- src/Paragraph.cpp (Revision 35252)
 +++ src/Paragraph.cpp (Arbeitskopie)
 @@ -73,8 +73,107 @@
  char_type const META_INSET = 0x21;
  };
  
 +
  /
  //
 +// Paragraph::SpellRanges
 +//
 +/
 +
 +class Paragraph::SpellCheckerState {

What is the real name? Does it need to be in Paragraph, or can it be in
Paragraph::Private? Or can it even be toplevel?

 +private:
 + /// store the ranges as map of FontSpan and spell result pairs
 + typedef mapFontSpan, SpellChecker::Result Ranges;
 + typedef mapFontSpan, SpellChecker::Result::const_iterator 
 RangesIterator;
 + Ranges ranges_;

typedef mapFontSpan, SpellChecker::Result Ranges;
typedef Ranges::const_iterator const_iterator;

 + void eraseCoveredRanges(FontSpan const fp)

 + void correctRangesAfterPos(pos_type pos, int offset)

I do not see why you need that. After all, the spellchecking is done
linearly in the paragraph, and thus all you have to do is to append to
the Range object (actually, a list and some push_back would be
enough).

I would have a std::list that lists the ranges for misspelled words.
Wouldn't that be enough?

 + Language * locateSpellRange(
 + pos_type  from, pos_type  to,
 + Positions  softbreaks) const;

Maybe:
Language * locateSpellRange(pos_type  from, pos_type  to,
Positions  softbreaks) const;

 + void setMisspelled(pos_type from, pos_type to, SpellChecker::Result 
 state)

As I wrote, this one is probably not needed either.

 @@ -297,6 +425,7 @@
   // Add a new entry in the fontlist_.
   fontlist_.set(fcit-pos() - beg, fcit-font());
   }
 + requestSpellCheck();
  }

I guess the question with this change and the others is whether you have
covered all the possibilities for modifying the paragraph. Time will
tell, I guess.

 + while (last  to  !owner_-isWordSeparator(last)) {
 + if (owner_-getInset(last)) {
 + softbreaks.insert(softbreaks.end(), last);
 + }
 + ++last;
 + }

Can you explain what the softbreaks variable is for?

 + /// start position and length of misspelled word at index
 + virtual void misspelledWord(
 + int index,
 + int  start, int  length) const
 + {
 + /// index is used here to make the compiler happy
 + if (index == 0)
 + start = 0;
 + length = 0;
 + }
 +

Why not make this misspelledWord method pure virtual?

BTW, If you want to avoid warnings about variables, all you have to do is:

virtual void misspelledWord(int /*index*/, int  /*start*/, 
int  /*length*/) const

Re: Patch for spell checker state in Paragraph instead of Font (Re: alpha6)

[Stephan Witt]

Am 01.09.2010 um 17:38 schrieb Jean-Marc LASGOUTTES:

 Stephan Witt st.w...@gmx.net writes:
 I've made some changes again... shorten lines as Jürgen suggested and
 formatting of if/while. And I added code to handle the soft-hyphens
 and friends.
 
 So I attach it again and hope I can put it in later. JMarc, can you
 have a look, please?
 
 See below. Nice work.

Thank you.

 
 +int AppleSpellChecker::numMisspelledWords() const
 +{
 +return numMisspelledWordsAppleSpeller(d-speller);
 +}
 
 It might be nice, short of having proper objc++ files, to rename the
 objc helper functions to something more obvious like
 AppleSpeller_numMisspelledWords (or even AS_numMisspelledWords).

Yes, you're right. I'll change the names.

 
 --- src/Text2.cpp(Revision 35252)
 +++ src/Text2.cpp(Arbeitskopie)
 @@ -356,6 +356,9 @@
  Font f = tm.displayFont(pit, pos);
  f.update(font, language, toggleall);
  setCharFont(pit, pos, f, tm.font_);
 +// font change may change language... 
 +// spell checker has to know that
 +pars_[pit].requestSpellCheck();
  }
 }
 
 
 
 -void setMisspelled(bool misspelled) { misspelled_ = misspelled; }
 -///
 -bool isMisspelled() const { return misspelled_; }
 -///
 
 Yay!
 
 
 Index: src/Paragraph.cpp
 ===
 --- src/Paragraph.cpp(Revision 35252)
 +++ src/Paragraph.cpp(Arbeitskopie)
 @@ -73,8 +73,107 @@
 char_type const META_INSET = 0x21;
 };
 
 +
 /
 //
 +// Paragraph::SpellRanges
 +//
 +/
 +
 +class Paragraph::SpellCheckerState {
 
 What is the real name? Does it need to be in Paragraph, or can it be in
 Paragraph::Private? Or can it even be toplevel?

It's all about two questions:
* does the paragraph need a check of spelling
* where are the error locations (ranges)

I thought, Paragraph::SpellCheckerState is private...

 
 +private:
 +/// store the ranges as map of FontSpan and spell result pairs
 +typedef mapFontSpan, SpellChecker::Result Ranges;
 +typedef mapFontSpan, SpellChecker::Result::const_iterator 
 RangesIterator;
 +Ranges ranges_;
 
   typedef mapFontSpan, SpellChecker::Result Ranges;
   typedef Ranges::const_iterator const_iterator;
 
 +void eraseCoveredRanges(FontSpan const fp)
 
 +void correctRangesAfterPos(pos_type pos, int offset)
 
 I do not see why you need that. After all, the spellchecking is done
 linearly in the paragraph, and thus all you have to do is to append to
 the Range object (actually, a list and some push_back would be
 enough).

My intention was to shift all positions to left or right on delete or insert of 
a char/inset.
This especially will be useful, when it comes to real spell check as you 
type. Then not the whole paragraph
needs to be checked again - only the word at the current position. But I'm not 
there...

As it is now, the flow of control is
* insert a char
* positions are corrected, paragraph spelling is marked dirty
* redraw paints the row and triggers the spell check
* the spell checker replaces the range items and 
* finds the errors again at the already corrected positions in list

 I would have a std::list that lists the ranges for misspelled words.
 Wouldn't that be enough?

I'm not so sophisticated with STL containers. :(
At some point I use the find() operation and std::list does not have it.
I have to admit - I was too lazy to try this as I considered this myself.

If the general approach is ok I'll try an alternative implementation with 
std::list.

 
 +Language * locateSpellRange(
 +pos_type  from, pos_type  to,
 +Positions  softbreaks) const;
 
 Maybe:
   Language * locateSpellRange(pos_type  from, pos_type  to,
   Positions  softbreaks) const;
 
 +void setMisspelled(pos_type from, pos_type to, SpellChecker::Result 
 state)
 
 As I wrote, this one is probably not needed either.

Why? It's in Paragraph::Private...
You mean the additional checks can go and I should call 
speller_state_.setRange() instead?

 
 @@ -297,6 +425,7 @@
  // Add a new entry in the fontlist_.
  fontlist_.set(fcit-pos() - beg, fcit-font());
  }
 +requestSpellCheck();
 }
 
 I guess the question with this change and the others is whether you have
 covered all the possibilities for modifying the paragraph. Time will
 tell, I guess.

Yes, I hope I have covered all - but Murphy will have the last word.
And this is exactly the biggest risk is this business.

 
 +while (last  to  !owner_-isWordSeparator(last)) {
 +if (owner_-getInset(last)) {
 +softbreaks.insert(softbreaks.end(), last);
 +}
 +++last;
 +}
 
 Can you 

Re: Patch for spell checker state in Paragraph instead of Font (Re: alpha6)

[Jean-Marc Lasgouttes]

Le 01/09/2010 20:11, Stephan Witt a écrit :

+// Paragraph::SpellRanges +//
+/



+

+class Paragraph::SpellCheckerState {


What is the real name? Does it need to be in Paragraph, or can it
be in Paragraph::Private? Or can it even be toplevel?


I meant that the comment and the class name do not match.


It's all about two questions: * does the paragraph need a check of
spelling * where are the error locations (ranges)

I thought, Paragraph::SpellCheckerState is private...


Yes, but there is no need to expose it in Paragraph.h. The class could
be standalone and a member speller_state_ still in Paragraph::Private.
This avoid friend declarations.


I do not see why you need that. After all, the spellchecking is
done linearly in the paragraph, and thus all you have to do is to
append to the Range object (actually, a list  and some push_back
would be enough).


My intention was to shift all positions to left or right on delete or
insert of a char/inset. This especially will be useful, when it comes
to real spell check as you type. Then not the whole paragraph needs
to be checked again - only the word at the current position. But I'm
not there...


I suspect this is overkill. Abdel's solution at row level was good in
this respect. Do you gain a lot in terms of performance when going at 
paragraph level. I fear this is going to become very fragile.


More later.

JMarc

Re: LyX 2.0 compilation errors using CMAKE

[Uwe Stöhr]

 using msvc this warning as handled as error.
 but it's fixed now.

Many thanks!

regards Uwe

Re: Patch for spell checker state in Paragraph instead of Font (Re: alpha6)

[Stephan Witt]

Am 02.09.2010 um 00:35 schrieb Jean-Marc Lasgouttes:

 Le 01/09/2010 20:11, Stephan Witt a écrit :
 +// Paragraph::SpellRanges +//
 +/
 
 
 +
 +class Paragraph::SpellCheckerState {
 
 What is the real name? Does it need to be in Paragraph, or can it
 be in Paragraph::Private? Or can it even be toplevel?
 
 I meant that the comment and the class name do not match.

The name in comment was from a previous implementation... I overlooked it.

 
 It's all about two questions: * does the paragraph need a check of
 spelling * where are the error locations (ranges)
 
 I thought, Paragraph::SpellCheckerState is private...
 
 Yes, but there is no need to expose it in Paragraph.h. The class could
 be standalone and a member speller_state_ still in Paragraph::Private.
 This avoid friend declarations.

Ok, that's a better solution. I haven't come up with this idea.

 
 I do not see why you need that. After all, the spellchecking is
 done linearly in the paragraph, and thus all you have to do is to
 append to the Range object (actually, a list  and some push_back
 would be enough).
 
 My intention was to shift all positions to left or right on delete or
 insert of a char/inset. This especially will be useful, when it comes
 to real spell check as you type. Then not the whole paragraph needs
 to be checked again - only the word at the current position. But I'm
 not there...
 
 I suspect this is overkill.
 Abdel's solution at row level was good in this respect.
 Do you gain a lot in terms of performance when going at paragraph level?

Yes, for scrolling - see below.

 I fear this is going to become very fragile.

That's why I stopped here and see currently no need to reduce the spell check 
range to 
the current word. 

Regarding the move to paragraph level check...
That simplifies the code and is my solution to get the F7-Spell check and 
scrolling right and fast.
To stay on row level means to rerun the spell check every time the (current) 
row changes.
When typing this is ok. But on scroll you have a complete spell check of the 
visible part again and again.
Now this is gone. I think that speeds up scrolling on all platforms. But I 
didn't profile this.

The point is to catch all possibilities to change paragraph contents. But that 
holds true for
change tracking either. Perhaps a common method can simplify that... But every 
change of content
has to go through Paragraph methods. So it must be doable with little effort.

Stephan

Re: LyX 2.0 compilation errors using CMAKE

[Stephan Witt]

Am 01.09.2010 um 08:01 schrieb Abdelrazak Younes:

 On 09/01/2010 01:38 AM, Uwe Stöhr wrote:
 
 However, this compilation error remains:
 
 10..\..\src\LyX.cpp(288) : error C4101: 'message' : unreferenced local 
 variable
 
 The attached patch fixes also this error for me (debug and release mode).
 
 It seems that MSVC doesn't allow to use empty catched exceptions.
 
 No, this is a warning about unused variabled but CMake treats these as error 
 (on purpose). This is to avoid to let unused variables everywhere like it 
 used to be.

Just for the record: the compiler flags CMake generates for Xcode are 
sub-optimal than.
I cannot see that type of warning with Xcode. I have to compile on Linux to 
catch them.
And the build with autotools spits out other warnings again. Not nice.
The compiler for Linux is gcc 4.4.1 and for MacOSX gcc 4.2.1.

Stephan

Re: LyX 2.0 compilation errors using CMAKE

[Abdelrazak Younes]

On 09/01/2010 01:38 AM, Uwe Stöhr wrote:

Am 31.08.2010 08:02, schrieb Stephan Witt:


I guess it's my change r35193.

Please try attached patch.


This fixes the setenv compilation errors -> I committed it.

However, this compilation error remains:

10>..\..\src\LyX.cpp(288) : error C4101: 'message' : unreferenced 
local variable


The attached patch fixes also this error for me (debug and release mode).


I am not sure about lyx_exit() at this stage of the boot process maybe 
just exist() will do. Also AFAIR Alert:: in LyX requires GUI mode so 
make sure your patch works with command-line mode (lyx -e).



It seems that MSVC doesn't allow to use empty catched exceptions.


No, this is a warning about unused variabled but CMake treats these as 
error (on purpose). This is to avoid to let unused variables everywhere 
like it used to be.



OK to put it in?


No, for now just comment out message:

-   } catch (ExceptionMessage const&  message) {}
+   } catch (ExceptionMessage const&  /*message*/) {


Abdel.

Re: LyX 2.0 compilation errors using CMAKE

[Abdelrazak Younes]

On 08/31/2010 03:15 AM, Uwe Stöhr wrote:

Am 31.08.2010 03:04, schrieb Uwe Stöhr:

I am not able to compile LyX 2.0 using CMAKE. I get these errors:


I forgot to say that I'm using
- Microsoft Visual C++ 2008 Express Edition with SP1
- Qt 4.6.3
- Windows XP 64bit (a.k.a. Windows 2003 Server)
- CMake 2.8.2

and build with this batch file:

-
:: set here the path to Qt's bin folder and to the LyX-MSVC depend bin 
folder

:: (these are example paths)
PATH=D:\Qt\bin;D:\LyXSVN\lyx-devel\lyx-windows-deps-msvc2008\bin;%PATH%

:: change directory to the folder where the compile result should be 
stored

:: (this is an example path)
cd D:\LyXSVN\lyx-devel\compile-result
cmake ..\development\cmake -G"Visual Studio 9 2008" -Dnls=1 
-DGNUWIN32_DIR=D:\LyXSVN\lyx-devel\lyx-windows-deps-msvc2008 -Dmerge=0


:: clean
start lyx.sln :: /clean Release

:: rebuild all generated files
::cmake ..\cmake -Dmerge_rebuild=1


First -Dmerge=0 and then -Dmerge_rebuild=1? This seems fishy...

In general when you are developping for debug, disable merge build. For 
release mode I agree that this is faster.


Abdel.

Re: lilybook module

[Guenter Milde]

On 2010-08-31, Julien Rioux wrote:
> On 31/08/2010 3:35 PM, Guenter Milde wrote:
>> On 2010-08-30, Jean-Marc Lasgouttes wrote:

>>> Using the OutputFormat tag defined for use with sweave, one can have a
>>> module that changes the format (and thus add an extra convert step).
...
>> Could this be used to allow LilyPond source code in LyX that is
>> converted to musical scores by the 'lilypond-book' pre-processor?

> Yes, I have already submitted a patch to this list to implement it.

Once again, there is a thread on the user list requesting this feature,
so I hope it can go e.g. to the Wiki.

> Since then I have worked on the patch a little bit. It works well except 
> for one or two things. As mentioned in a previous message in this tread, 
> instant preview always treats the current buffer as LaTeX, so any 
> attempt to change the OutputFormat to allow preprocessing by 
> lilypond-book is ignored by the preview mechanism. My current fix to get 
> Instant preview working is kind of a hack, so I haven't posted a updated 
> patch yet.

For me, it would be OK to typeset the LilyPond code verbatim in the
instant preview with a macro redefinition in the preamble that gets
removed/replaced by the pre-processor. I agree this is not WYSIWYG but an
instant fallback solution.

Günter

Re: LyX 2.0 compilation errors using CMAKE

[Peter Kümmel]

On 01.09.2010 01:38, Uwe Stöhr wrote:
> Am 31.08.2010 08:02, schrieb Stephan Witt:
> 
>> I guess it's my change r35193.
>>
>> Please try attached patch.
> 
> This fixes the setenv compilation errors -> I committed it.
> 
> However, this compilation error remains:
> 
> 10>..\..\src\LyX.cpp(288) : error C4101: 'message' : unreferenced local 
> variable

using msvc this warning as handled as error.
but it's fixed now.

> 
> The attached patch fixes also this error for me (debug and release mode). It 
> seems that MSVC doesn't 
> allow to use empty catched exceptions. OK to put it in?
> 
> thanks and regards
> Uwe

Re: LyX 2.0 compilation errors using CMAKE

[Peter Kümmel]

> First -Dmerge=0 and then -Dmerge_rebuild=1? This seems fishy...
> 
> In general when you are developping for debug, disable merge build. For 
> release mode I agree that this is faster.
> 
> Abdel.
> 

these options are obsolete now. all options now start with LYX_ see cmake output
or cmake-gui.

I've also update development/cmake/build.bat. Hope the output is self 
explaining:

-
Usage build.bat devel/install/deploy STUDIO(optional)
devel   - Builds Visual Studio project files for development on LyX
install - Builds Visual Studio project files with all enabled for 
installation
deploy  - Builds Makefiles and installs LyX
STUDIO  - Used Visual Studio version, default is "Visual Studio 9 2008"
  use "Visual Studio 10" for Visual Studio 10
-
Be sure you've set qmake in PATH and set the variables:
GNUWIN32_DIR
LYX_SOURCE
LXY_BUILD
Or edit this file.
-
ERROR: no options.
Exiting now.


Peter

Re: Patch for spell checker state in Paragraph instead of Font (Re: alpha6)

[Jean-Marc LASGOUTTES]

Stephan Witt  writes:
> I've made some changes again... shorten lines as Jürgen suggested and
> formatting of if/while. And I added code to handle the soft-hyphens
> and friends.
>
> So I attach it again and hope I can put it in later. JMarc, can you
> have a look, please?

See below. Nice work.

JMarc

> +int AppleSpellChecker::numMisspelledWords() const
> +{
> + return numMisspelledWordsAppleSpeller(d->speller);
> +}

It might be nice, short of having proper objc++ files, to rename the
objc helper functions to something more obvious like
AppleSpeller_numMisspelledWords (or even AS_numMisspelledWords).

> --- src/Text2.cpp (Revision 35252)
> +++ src/Text2.cpp (Arbeitskopie)
> @@ -356,6 +356,9 @@
>   Font f = tm.displayFont(pit, pos);
>   f.update(font, language, toggleall);
>   setCharFont(pit, pos, f, tm.font_);
> + // font change may change language... 
> + // spell checker has to know that
> + pars_[pit].requestSpellCheck();
>   }
>  }
>  


> - void setMisspelled(bool misspelled) { misspelled_ = misspelled; }
> - ///
> - bool isMisspelled() const { return misspelled_; }
> - ///

Yay!


> Index: src/Paragraph.cpp
> ===
> --- src/Paragraph.cpp (Revision 35252)
> +++ src/Paragraph.cpp (Arbeitskopie)
> @@ -73,8 +73,107 @@
>  char_type const META_INSET = 0x21;
>  };
>  
> +
>  /
>  //
> +// Paragraph::SpellRanges
> +//
> +/
> +
> +class Paragraph::SpellCheckerState {

What is the real name? Does it need to be in Paragraph, or can it be in
Paragraph::Private? Or can it even be toplevel?

> +private:
> + /// store the ranges as map of FontSpan and spell result pairs
> + typedef map Ranges;
> + typedef map::const_iterator 
> RangesIterator;
> + Ranges ranges_;

typedef map Ranges;
typedef Ranges::const_iterator const_iterator;

> + void eraseCoveredRanges(FontSpan const fp)

> + void correctRangesAfterPos(pos_type pos, int offset)

I do not see why you need that. After all, the spellchecking is done
linearly in the paragraph, and thus all you have to do is to append to
the Range object (actually, a list<> and some push_back would be
enough).

I would have a std::list that lists the ranges for misspelled words.
Wouldn't that be enough?

> + Language * locateSpellRange(
> + pos_type & from, pos_type & to,
> + Positions & softbreaks) const;

Maybe:
Language * locateSpellRange(pos_type & from, pos_type & to,
Positions & softbreaks) const;

> + void setMisspelled(pos_type from, pos_type to, SpellChecker::Result 
> state)

As I wrote, this one is probably not needed either.

> @@ -297,6 +425,7 @@
>   // Add a new entry in the fontlist_.
>   fontlist_.set(fcit->pos() - beg, fcit->font());
>   }
> + requestSpellCheck();
>  }

I guess the question with this change and the others is whether you have
covered all the possibilities for modifying the paragraph. Time will
tell, I guess.

> + while (last < to && !owner_->isWordSeparator(last)) {
> + if (owner_->getInset(last)) {
> + softbreaks.insert(softbreaks.end(), last);
> + }
> + ++last;
> + }

Can you explain what the softbreaks variable is for?

> + /// start position and length of misspelled word at index
> + virtual void misspelledWord(
> + int index,
> + int & start, int & length) const
> + {
> + /// index is used here to make the compiler happy
> + if (index == 0)
> + start = 0;
> + length = 0;
> + }
> +

Why not make this misspelledWord method pure virtual?

BTW, If you want to avoid warnings about variables, all you have to do is:

virtual void misspelledWord(int /*index*/, int & /*start*/, 
int & /*length*/) const

Re: Patch for spell checker state in Paragraph instead of Font (Re: alpha6)

[Stephan Witt]

Am 01.09.2010 um 17:38 schrieb Jean-Marc LASGOUTTES:

> Stephan Witt  writes:
>> I've made some changes again... shorten lines as Jürgen suggested and
>> formatting of if/while. And I added code to handle the soft-hyphens
>> and friends.
>> 
>> So I attach it again and hope I can put it in later. JMarc, can you
>> have a look, please?
> 
> See below. Nice work.

Thank you.

> 
>> +int AppleSpellChecker::numMisspelledWords() const
>> +{
>> +return numMisspelledWordsAppleSpeller(d->speller);
>> +}
> 
> It might be nice, short of having proper objc++ files, to rename the
> objc helper functions to something more obvious like
> AppleSpeller_numMisspelledWords (or even AS_numMisspelledWords).

Yes, you're right. I'll change the names.

> 
>> --- src/Text2.cpp(Revision 35252)
>> +++ src/Text2.cpp(Arbeitskopie)
>> @@ -356,6 +356,9 @@
>>  Font f = tm.displayFont(pit, pos);
>>  f.update(font, language, toggleall);
>>  setCharFont(pit, pos, f, tm.font_);
>> +// font change may change language... 
>> +// spell checker has to know that
>> +pars_[pit].requestSpellCheck();
>>  }
>> }
>> 
> 
> 
>> -void setMisspelled(bool misspelled) { misspelled_ = misspelled; }
>> -///
>> -bool isMisspelled() const { return misspelled_; }
>> -///
> 
> Yay!
> 
> 
>> Index: src/Paragraph.cpp
>> ===
>> --- src/Paragraph.cpp(Revision 35252)
>> +++ src/Paragraph.cpp(Arbeitskopie)
>> @@ -73,8 +73,107 @@
>> char_type const META_INSET = 0x21;
>> };
>> 
>> +
>> /
>> //
>> +// Paragraph::SpellRanges
>> +//
>> +/
>> +
>> +class Paragraph::SpellCheckerState {
> 
> What is the real name? Does it need to be in Paragraph, or can it be in
> Paragraph::Private? Or can it even be toplevel?

It's all about two questions:
* does the paragraph need a check of spelling
* where are the error locations (ranges)

I thought, Paragraph::SpellCheckerState is private...

> 
>> +private:
>> +/// store the ranges as map of FontSpan and spell result pairs
>> +typedef map Ranges;
>> +typedef map::const_iterator 
>> RangesIterator;
>> +Ranges ranges_;
> 
>   typedef map Ranges;
>   typedef Ranges::const_iterator const_iterator;
> 
>> +void eraseCoveredRanges(FontSpan const fp)
> 
>> +void correctRangesAfterPos(pos_type pos, int offset)
> 
> I do not see why you need that. After all, the spellchecking is done
> linearly in the paragraph, and thus all you have to do is to append to
> the Range object (actually, a list<> and some push_back would be
> enough).

My intention was to shift all positions to left or right on delete or insert of 
a char/inset.
This especially will be useful, when it comes to real "spell check as you 
type". Then not the whole paragraph
needs to be checked again - only the word at the current position. But I'm not 
there...

As it is now, the flow of control is
* insert a char
* positions are corrected, paragraph spelling is marked dirty
* redraw paints the row and triggers the spell check
* the spell checker replaces the range items and 
* finds the errors again at the already corrected positions in list

> I would have a std::list that lists the ranges for misspelled words.
> Wouldn't that be enough?

I'm not so sophisticated with STL containers. :(
At some point I use the find() operation and std::list does not have it.
I have to admit - I was too lazy to try this as I considered this myself.

If the general approach is ok I'll try an alternative implementation with 
std::list.

> 
>> +Language * locateSpellRange(
>> +pos_type & from, pos_type & to,
>> +Positions & softbreaks) const;
> 
> Maybe:
>   Language * locateSpellRange(pos_type & from, pos_type & to,
>   Positions & softbreaks) const;
> 
>> +void setMisspelled(pos_type from, pos_type to, SpellChecker::Result 
>> state)
> 
> As I wrote, this one is probably not needed either.

Why? It's in Paragraph::Private...
You mean the additional checks can go and I should call 
speller_state_.setRange() instead?

> 
>> @@ -297,6 +425,7 @@
>>  // Add a new entry in the fontlist_.
>>  fontlist_.set(fcit->pos() - beg, fcit->font());
>>  }
>> +requestSpellCheck();
>> }
> 
> I guess the question with this change and the others is whether you have
> covered all the possibilities for modifying the paragraph. Time will
> tell, I guess.

Yes, I hope I have covered all - but Murphy will have the last word.
And this is exactly the biggest risk is this business.

> 
>> +while (last < to && !owner_->isWordSeparator(last)) {
>> +

Re: Patch for spell checker state in Paragraph instead of Font (Re: alpha6)

[Jean-Marc Lasgouttes]

Le 01/09/2010 20:11, Stephan Witt a écrit :

+// Paragraph::SpellRanges +//
+/



+

+class Paragraph::SpellCheckerState {


What is the real name? Does it need to be in Paragraph, or can it
be in Paragraph::Private? Or can it even be toplevel?


I meant that the comment and the class name do not match.


It's all about two questions: * does the paragraph need a check of
spelling * where are the error locations (ranges)

I thought, Paragraph::SpellCheckerState is private...


Yes, but there is no need to expose it in Paragraph.h. The class could
be standalone and a member speller_state_ still in Paragraph::Private.
This avoid friend declarations.


I do not see why you need that. After all, the spellchecking is
done linearly in the paragraph, and thus all you have to do is to
append to the Range object (actually, a list<>  and some push_back
would be enough).


My intention was to shift all positions to left or right on delete or
insert of a char/inset. This especially will be useful, when it comes
to real "spell check as you type". Then not the whole paragraph needs
to be checked again - only the word at the current position. But I'm
not there...


I suspect this is overkill. Abdel's solution at row level was good in
this respect. Do you gain a lot in terms of performance when going at 
paragraph level. I fear this is going to become very fragile.


More later.

JMarc

Re: LyX 2.0 compilation errors using CMAKE

[Uwe Stöhr]

> using msvc this warning as handled as error.
> but it's fixed now.

Many thanks!

regards Uwe

Re: Patch for spell checker state in Paragraph instead of Font (Re: alpha6)

[Stephan Witt]

Am 02.09.2010 um 00:35 schrieb Jean-Marc Lasgouttes:

> Le 01/09/2010 20:11, Stephan Witt a écrit :
 +// Paragraph::SpellRanges +//
 +/
 
 
> +
 +class Paragraph::SpellCheckerState {
>>> 
>>> What is the real name? Does it need to be in Paragraph, or can it
>>> be in Paragraph::Private? Or can it even be toplevel?
> 
> I meant that the comment and the class name do not match.

The name in comment was from a previous implementation... I overlooked it.

> 
>> It's all about two questions: * does the paragraph need a check of
>> spelling * where are the error locations (ranges)
>> 
>> I thought, Paragraph::SpellCheckerState is private...
> 
> Yes, but there is no need to expose it in Paragraph.h. The class could
> be standalone and a member speller_state_ still in Paragraph::Private.
> This avoid friend declarations.

Ok, that's a better solution. I haven't come up with this idea.

> 
>>> I do not see why you need that. After all, the spellchecking is
>>> done linearly in the paragraph, and thus all you have to do is to
>>> append to the Range object (actually, a list<>  and some push_back
>>> would be enough).
>> 
>> My intention was to shift all positions to left or right on delete or
>> insert of a char/inset. This especially will be useful, when it comes
>> to real "spell check as you type". Then not the whole paragraph needs
>> to be checked again - only the word at the current position. But I'm
>> not there...
> 
> I suspect this is overkill.
> Abdel's solution at row level was good in this respect.
> Do you gain a lot in terms of performance when going at paragraph level?

Yes, for scrolling - see below.

> I fear this is going to become very fragile.

That's why I stopped here and see currently no need to reduce the spell check 
range to 
the current word. 

Regarding the move to paragraph level check...
That simplifies the code and is my solution to get the F7-Spell check and 
scrolling right and fast.
To stay on row level means to rerun the spell check every time the (current) 
row changes.
When typing this is ok. But on scroll you have a complete spell check of the 
visible part again and again.
Now this is gone. I think that speeds up scrolling on all platforms. But I 
didn't profile this.

The point is to catch all possibilities to change paragraph contents. But that 
holds true for
change tracking either. Perhaps a common method can simplify that... But every 
change of content
has to go through Paragraph methods. So it must be doable with little effort.

Stephan

Re: LyX 2.0 compilation errors using CMAKE

[Stephan Witt]

Am 01.09.2010 um 08:01 schrieb Abdelrazak Younes:

> On 09/01/2010 01:38 AM, Uwe Stöhr wrote:
>> 
>> However, this compilation error remains:
>> 
>> 10>..\..\src\LyX.cpp(288) : error C4101: 'message' : unreferenced local 
>> variable
>> 
>> The attached patch fixes also this error for me (debug and release mode).
> 
>> It seems that MSVC doesn't allow to use empty catched exceptions.
> 
> No, this is a warning about unused variabled but CMake treats these as error 
> (on purpose). This is to avoid to let unused variables everywhere like it 
> used to be.

Just for the record: the compiler flags CMake generates for Xcode are 
sub-optimal than.
I cannot see that type of warning with Xcode. I have to compile on Linux to 
catch them.
And the build with autotools spits out other warnings again. Not nice.
The compiler for Linux is gcc 4.4.1 and for MacOSX gcc 4.2.1.

Stephan