Re: Description of trac component 'frontend-qt4' ?

[Jürgen Spitzmüller]

2015-05-07 21:55 GMT+02:00 Richard Heck rgh...@lyx.org:

 What would be nice is just to mothball dialog. But I don't see any way
 to disable it without just removing it. Though we could do that, and
 mass-move them to frontend-qt4 (or just: frontend).


Note, though, that the disctinction of frontend-qt4 and -qt5 is quite
useful at the moment.

I would assign to dialogs all things that have really to do with the dialog
design (ui, ux), to frontend the stuff specific to a particular toolkit
(qt4, qt5) and the non dialog frontend stuff.

Jürgen

Re: [LyX/master] #9514 improved document iterator for correct check of speller disabled state

[Kornel Benko]

Am Freitag, 8. Mai 2015 um 11:13:57, schrieb Stephan Witt sw...@lyx.org
 commit e93444e7e9d6f1aeabd356d414d4e5af3cea8569
 Author: Stephan Witt sw...@lyx.org
 Date:   Fri May 8 11:13:32 2015 +0200
 
 #9514 improved document iterator for correct check of speller disabled 
 state
 

I cannot compile ATM.

[ 77%] Building CXX object src/CMakeFiles/lyx2.2.dir/RowPainter.cpp.o
cd /usr/BUILD/BuildLyxGitQt5/src  /usr/bin/c++   
-DBOOST_SIGNALS_NO_DEPRECATION_WARNING=1 -DQT_CONCURRENT_LIB -DQT_CORE_LIB 
-DQT_GUI_LIB -DQT_WIDGETS_LIB -Wall -Wunused-parameter --std=gnu++11 
-fno-strict-aliasing  -Wall -Wunused-parameter --std=gnu++11 
-fno-strict-aliasing -O0 -g3 -D_DEBUG -fPIE -I/usr/BUILD/BuildLyxGitQt5 
-I/usr/src/lyx/lyx-git/src -I/usr/include/enchant -I/usr/src/lyx/lyx-git/boost 
-I/usr/BUILD/BuildLyxGitQt5/src -isystem /usr/BUILD/BuildQt5/5.4/gcc_64/include 
-isystem /usr/BUILD/BuildQt5/5.4/gcc_64/include/QtCore -isystem 
/usr/BUILD/BuildQt5/5.4/gcc_64/mkspecs/linux-g++ -isystem 
/usr/BUILD/BuildQt5/5.4/gcc_64/include/QtGui -isystem 
/usr/BUILD/BuildQt5/5.4/gcc_64/include/QtWidgets -isystem 
/usr/BUILD/BuildQt5/5.4/gcc_64/include/QtConcurrent
-DBOOST_USER_CONFIG=config.h -o CMakeFiles/lyx2.2.dir/RowPainter.cpp.o -c 
/usr/src/lyx/lyx-git/src/RowPainter.cpp
/usr/src/lyx/lyx-git/src/RowPainter.cpp: In member function ‘void 
lyx::RowPainter::paintFromPos(lyx::pos_type, bool)’:
/usr/src/lyx/lyx-git/src/RowPainter.cpp:334:45: error: no matching function for 
call to ‘lyx::RowPainter::paintMisspelledMark(const double, bool, const 
lyx::Font)’
paintMisspelledMark(orig_x, changed, font);
 ^
/usr/src/lyx/lyx-git/src/RowPainter.cpp:334:45: note: candidate is:
/usr/src/lyx/lyx-git/src/RowPainter.cpp:296:6: note: void 
lyx::RowPainter::paintMisspelledMark(double, bool)
 void RowPainter::paintMisspelledMark(double orig_x, bool changed)
  ^
/usr/src/lyx/lyx-git/src/RowPainter.cpp:296:6: note:   candidate expects 2 
arguments, 3 provided


Kornel

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

Re: [LyX/master] Fix compilation after e93444e7

[Stephan Witt]

Am 08.05.2015 um 14:17 schrieb Enrico Forestieri for...@lyx.org:

 commit bc8717b297e0b3001ccaf5146ba5ec945c77ad42
 Author: Enrico Forestieri for...@lyx.org
 Date:   Fri May 8 14:15:38 2015 +0200
 
 Fix compilation after e93444e7
 
 Stephan, please check whether RowPainter::paintMisspelledMark has
 to be extended to use a third parameter, instead.
 
 diff --git a/src/RowPainter.cpp b/src/RowPainter.cpp
 index d556cfa..1b7c33d 100644
 --- a/src/RowPainter.cpp
 +++ b/src/RowPainter.cpp
 @@ -331,7 +331,7 @@ void RowPainter::paintFromPos(pos_type  vpos, bool 
 changed)
   new_word = par_.isSameSpellRange(pos, cpos) ;
   }
   if (!new_word  pi_.do_spellcheck)
 - paintMisspelledMark(orig_x, changed, font);
 + paintMisspelledMark(orig_x, changed);
   }
 }
 

Sorry, the hunk was a mixture of my change and a patch from JMarc he sent to 
correct the thickness of the misspelled marker. I overlooked that and your 
change is the correct one, thanks.

Stephan

Re: Failure to compile lyx with gcc 5.1 with the new ABI

[Georg Baum]

José Matos wrote:

 While testing what packages failed with the new ABI one of the cases is
 LyX (2.1.3), the first builder to fail was the x86_64 one and this is why
 this message refers to it:
 
 https://kojipkgs.fedoraproject.org//work/tasks/1267/9651267/build.log
 
 The failures start at the begin, so there is no need to go down to find
 the culprit.
 
 Any idea about what is wrong here?

The forward declarations in src/support/strfwd.h were wrong. I fixed this by 
not using the forward declarations in C++11 mode and including string 
instead. Feel free to adapt the forward declarations if you prefer that, the 
configure machinery is now in place. I hesitated to include libstdc++ 
interna in LyX sources, therefore I did not do it myself.

I guess this is also a candidate for branch, but I would prefer a review for 
the autotools and cmake stuff before.


Georg

C++ question about auto_ptr

[Jean-Marc Lasgouttes]

With C++11, auto_ptr is deprecated and we get warnings.

I am trying to see how we can get rid of it. unique_ptr is new to C++11, 
so I'd rather avoid that.


I have many places like in the patch below. Is there a reason why I 
should keep the auto_ptr instead of a naked pointer? What is it good for?


JMarc
diff --git a/src/BufferList.cpp b/src/BufferList.cpp
index 68a1e80..724c7dd 100644
--- a/src/BufferList.cpp
+++ b/src/BufferList.cpp
@@ -130,9 +130,9 @@ Buffer * BufferList::newBuffer(string const  s)
 
 Buffer * BufferList::createNewBuffer(string const  s)
 {
-	auto_ptrBuffer tmpbuf;
+	Buffer * tmpbuf;
 	try {
-		tmpbuf.reset(new Buffer(s));
+		tmpbuf = new Buffer(s);
 	} catch (ExceptionMessage const  message) {
 		if (message.type_ == ErrorException) {
 			Alert::error(message.title_, message.details_);
@@ -143,7 +143,7 @@ Buffer * BufferList::createNewBuffer(string const  s)
 		}
 	}
 	tmpbuf-params().useClassDefaults();
-	return tmpbuf.release();
+	return tmpbuf;
 }
 
 

Re: C++ question about auto_ptr

[Georg Baum]

Jean-Marc Lasgouttes wrote:

 With C++11, auto_ptr is deprecated and we get warnings.
 
 I am trying to see how we can get rid of it. unique_ptr is new to C++11,
 so I'd rather avoid that.

Why not use unique_ptr fpr C++11 and auto_ptr else? The difference is in the 
copying semantics, which does not matter for the current use cases. The only 
challenge would be to find a good name fo the typedef.

 I have many places like in the patch below. Is there a reason why I
 should keep the auto_ptr instead of a naked pointer? What is it good for?

Usually it is used for exception safety: If you use a raw pointer, you need 
to delete it in the catch clause, else you get a memory leak. This would be 
easy to do in this particular example, but impossible if the catch happens 
somewhere completely different.


Georg

Wrong enum as boolean usage in Text3.cpp

[Stephan Witt]

Since change 02cb5fd3 one can see the clever code fragment in Text3.cpp line 
2934..2943 has a problem with Clipboard::PdfGraphicsType. The assignment of 
type for pdf at line 2935 is not true for arg value pdf. The enum 
Clipboard::GraphicsType shouldn't start with PdfGraphicsType or the code block 
has to be changed. Solution 1 is attached as patch. Ok to apply?

Stephan


PdfGraphicsType.patch
Description: Binary data

Re: Wrong enum as boolean usage in Text3.cpp

[Scott Kostyshak]

On Fri, May 8, 2015 at 10:29 PM, Stephan Witt st.w...@gmx.net wrote:
 Since change 02cb5fd3 one can see the clever code fragment in Text3.cpp line 
 2934..2943 has a problem with Clipboard::PdfGraphicsType. The assignment of 
 type for pdf at line 2935 is not true for arg value pdf. The enum 
 Clipboard::GraphicsType shouldn't start with PdfGraphicsType or the code 
 block has to be changed. Solution 1 is attached as patch. Ok to apply?

I was just working on the same thing. I imagine you looked into it
because of the message on the terminal also? In any case, my patch is
attached.

I do not like the approach you propose because if the code is changed
around we could see the same problem. I don't have a realistic idea in
mind for that though. Actually, it's not that I don't like the
approach you took, I don't like that code style for the if/if-else. It
just seems like going a little too much out of the way to save
vertical space. In some sense, I admit that your patch is the least
invasive because it is a small patch.

Does that code style lead to an optimization somehow?

I don't have a strong opinion on this. However, if you like your patch
better than the one I propose, would it be OK to define the top one to
be a number greater than 0?

Scott
From c7ce0bb2e1ffbc5b492d800aca82181d33f03cfa Mon Sep 17 00:00:00 2001
From: Scott Kostyshak skost...@princeton.edu
Date: Fri, 8 May 2015 19:31:47 -0400
Subject: [PATCH] Fix pasting of PDF from clipboard

The command paste pdf was always disabled because the
condition in the following if statement always returns false

  if (arg == pdf  (type = Clipboard::PdfGraphicsType))

The value of type is zero in this case because PdfGraphicsType is
the first enum value (and it is not set explicitly to non-zero).

An alternative patch is to put AnyGraphicsType as the first
element of enum GraphicsType, or to set the first element to a
number greater than 0.

To test the bug that this commit fixes, either copy a PDF and try to
paste with the action paste pdf, or click on the Edit menu and
notice (before this commit) the terminal output Unrecognized
graphics type: pdf.
---
 src/Text3.cpp | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/Text3.cpp b/src/Text3.cpp
index f80bde6..dbff72a 100644
--- a/src/Text3.cpp
+++ b/src/Text3.cpp
@@ -2930,25 +2930,30 @@ bool Text::getStatus(Cursor  cur, FuncRequest const  cmd,
 			break;
 		}
 
-		// explicit graphics type?
 		Clipboard::GraphicsType type = Clipboard::AnyGraphicsType;
-		if ((arg == pdf  (type = Clipboard::PdfGraphicsType))
-			  || (arg == png  (type = Clipboard::PngGraphicsType))
-			  || (arg == jpeg  (type = Clipboard::JpegGraphicsType))
-			  || (arg == linkback   (type = Clipboard::LinkBackGraphicsType))
-			  || (arg == emf   (type = Clipboard::EmfGraphicsType))
-			  || (arg == wmf   (type = Clipboard::WmfGraphicsType))) {
-			enable = theClipboard().hasGraphicsContents(type);
+		if (arg == pdf)
+			type = Clipboard::PdfGraphicsType;
+		else if (arg == png)
+			type = Clipboard::PngGraphicsType;
+		else if (arg == jpeg)
+			type = Clipboard::JpegGraphicsType;
+		else if (arg == linkback)
+			type = Clipboard::LinkBackGraphicsType;
+		else if (arg == emf)
+			type = Clipboard::EmfGraphicsType;
+		else if (arg == wmf)
+			type = Clipboard::WmfGraphicsType;
+		else {
+			// unknown argument
+			LYXERR0(Unrecognized graphics type:   arg);
+			// we don't want to assert if the user just mistyped the LFUN
+			LATTEST(cmd.origin() != FuncRequest::INTERNAL);
+			enable = false;
 			break;
 		}
-
-		// unknown argument
-		LYXERR0(Unrecognized graphics type:   arg);
-		// we don't want to assert if the user just mistyped the LFUN
-		LATTEST(cmd.origin() != FuncRequest::INTERNAL);
-		enable = false;
+		enable = theClipboard().hasGraphicsContents(type);
 		break;
-	 }
+	}
 
 	case LFUN_CLIPBOARD_PASTE:
 	case LFUN_CLIPBOARD_PASTE_SIMPLE:
-- 
2.1.4

Re: Wrong enum as boolean usage in Text3.cpp

[Stephan Witt]

Am 09.05.2015 um 05:12 schrieb Scott Kostyshak skost...@lyx.org:

 On Fri, May 8, 2015 at 10:29 PM, Stephan Witt st.w...@gmx.net wrote:
 Since change 02cb5fd3 one can see the clever code fragment in Text3.cpp line 
 2934..2943 has a problem with Clipboard::PdfGraphicsType. The assignment of 
 type for pdf at line 2935 is not true for arg value pdf. The enum 
 Clipboard::GraphicsType shouldn't start with PdfGraphicsType or the code 
 block has to be changed. Solution 1 is attached as patch. Ok to apply?
 
 I was just working on the same thing. I imagine you looked into it
 because of the message on the terminal also? In any case, my patch is
 attached.

This one is obviously more readable and you should apply your patch.

 I do not like the approach you propose because if the code is changed
 around we could see the same problem. I don't have a realistic idea in
 mind for that though. Actually, it's not that I don't like the
 approach you took, I don't like that code style for the if/if-else. It
 just seems like going a little too much out of the way to save
 vertical space. In some sense, I admit that your patch is the least
 invasive because it is a small patch.
 
 Does that code style lead to an optimization somehow?
 
 I don't have a strong opinion on this. However, if you like your patch
 better than the one I propose, would it be OK to define the top one to
 be a number greater than 0?

This solution I've tried too and it would work. But the next enum in 
Clipboard.h (TextType) starts with an Any type too and so I made them more 
similar. I prefer this for enums like that because extending them doesn't 
change the existing values. Of course this is not really important.

Stephan

Re: Description of trac component 'frontend-qt4' ?

[Jürgen Spitzmüller]

2015-05-07 21:55 GMT+02:00 Richard Heck :

> What would be nice is just to mothball "dialog". But I don't see any way
> to disable it without just removing it. Though we could do that, and
> mass-move them to frontend-qt4 (or just: frontend).
>

Note, though, that the disctinction of frontend-qt4 and -qt5 is quite
useful at the moment.

I would assign to dialogs all things that have really to do with the dialog
design (ui, ux), to frontend the stuff specific to a particular toolkit
(qt4, qt5) and the non dialog frontend stuff.

Jürgen

Re: [LyX/master] #9514 improved document iterator for correct check of speller disabled state

[Kornel Benko]

Am Freitag, 8. Mai 2015 um 11:13:57, schrieb Stephan Witt 
> commit e93444e7e9d6f1aeabd356d414d4e5af3cea8569
> Author: Stephan Witt 
> Date:   Fri May 8 11:13:32 2015 +0200
> 
> #9514 improved document iterator for correct check of speller disabled 
> state
> 

I cannot compile ATM.

[ 77%] Building CXX object src/CMakeFiles/lyx2.2.dir/RowPainter.cpp.o
cd /usr/BUILD/BuildLyxGitQt5/src && /usr/bin/c++   
-DBOOST_SIGNALS_NO_DEPRECATION_WARNING=1 -DQT_CONCURRENT_LIB -DQT_CORE_LIB 
-DQT_GUI_LIB -DQT_WIDGETS_LIB -Wall -Wunused-parameter --std=gnu++11 
-fno-strict-aliasing  -Wall -Wunused-parameter --std=gnu++11 
-fno-strict-aliasing -O0 -g3 -D_DEBUG -fPIE -I/usr/BUILD/BuildLyxGitQt5 
-I/usr/src/lyx/lyx-git/src -I/usr/include/enchant -I/usr/src/lyx/lyx-git/boost 
-I/usr/BUILD/BuildLyxGitQt5/src -isystem /usr/BUILD/BuildQt5/5.4/gcc_64/include 
-isystem /usr/BUILD/BuildQt5/5.4/gcc_64/include/QtCore -isystem 
/usr/BUILD/BuildQt5/5.4/gcc_64/mkspecs/linux-g++ -isystem 
/usr/BUILD/BuildQt5/5.4/gcc_64/include/QtGui -isystem 
/usr/BUILD/BuildQt5/5.4/gcc_64/include/QtWidgets -isystem 
/usr/BUILD/BuildQt5/5.4/gcc_64/include/QtConcurrent
-DBOOST_USER_CONFIG="" -o CMakeFiles/lyx2.2.dir/RowPainter.cpp.o -c 
/usr/src/lyx/lyx-git/src/RowPainter.cpp
/usr/src/lyx/lyx-git/src/RowPainter.cpp: In member function ‘void 
lyx::RowPainter::paintFromPos(lyx::pos_type&, bool)’:
/usr/src/lyx/lyx-git/src/RowPainter.cpp:334:45: error: no matching function for 
call to ‘lyx::RowPainter::paintMisspelledMark(const double&, bool&, const 
lyx::Font&)’
paintMisspelledMark(orig_x, changed, font);
 ^
/usr/src/lyx/lyx-git/src/RowPainter.cpp:334:45: note: candidate is:
/usr/src/lyx/lyx-git/src/RowPainter.cpp:296:6: note: void 
lyx::RowPainter::paintMisspelledMark(double, bool)
 void RowPainter::paintMisspelledMark(double orig_x, bool changed)
  ^
/usr/src/lyx/lyx-git/src/RowPainter.cpp:296:6: note:   candidate expects 2 
arguments, 3 provided


Kornel

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

Re: [LyX/master] Fix compilation after e93444e7

[Stephan Witt]

Am 08.05.2015 um 14:17 schrieb Enrico Forestieri :

> commit bc8717b297e0b3001ccaf5146ba5ec945c77ad42
> Author: Enrico Forestieri 
> Date:   Fri May 8 14:15:38 2015 +0200
> 
> Fix compilation after e93444e7
> 
> Stephan, please check whether RowPainter::paintMisspelledMark has
> to be extended to use a third parameter, instead.
> 
> diff --git a/src/RowPainter.cpp b/src/RowPainter.cpp
> index d556cfa..1b7c33d 100644
> --- a/src/RowPainter.cpp
> +++ b/src/RowPainter.cpp
> @@ -331,7 +331,7 @@ void RowPainter::paintFromPos(pos_type & vpos, bool 
> changed)
>   new_word = par_.isSameSpellRange(pos, cpos) ;
>   }
>   if (!new_word && pi_.do_spellcheck)
> - paintMisspelledMark(orig_x, changed, font);
> + paintMisspelledMark(orig_x, changed);
>   }
> }
> 

Sorry, the hunk was a mixture of my change and a patch from JMarc he sent to 
correct the thickness of the misspelled marker. I overlooked that and your 
change is the correct one, thanks.

Stephan

Re: Failure to compile lyx with gcc 5.1 with the new ABI

[Georg Baum]

José Matos wrote:

> While testing what packages failed with the new ABI one of the cases is
> LyX (2.1.3), the first builder to fail was the x86_64 one and this is why
> this message refers to it:
> 
> https://kojipkgs.fedoraproject.org//work/tasks/1267/9651267/build.log
> 
> The failures start at the begin, so there is no need to go down to find
> the culprit.
> 
> Any idea about what is wrong here?

The forward declarations in src/support/strfwd.h were wrong. I fixed this by 
not using the forward declarations in C++11 mode and including  
instead. Feel free to adapt the forward declarations if you prefer that, the 
configure machinery is now in place. I hesitated to include libstdc++ 
interna in LyX sources, therefore I did not do it myself.

I guess this is also a candidate for branch, but I would prefer a review for 
the autotools and cmake stuff before.


Georg

C++ question about auto_ptr

[Jean-Marc Lasgouttes]

With C++11, auto_ptr is deprecated and we get warnings.

I am trying to see how we can get rid of it. unique_ptr is new to C++11, 
so I'd rather avoid that.


I have many places like in the patch below. Is there a reason why I 
should keep the auto_ptr instead of a naked pointer? What is it good for?


JMarc
diff --git a/src/BufferList.cpp b/src/BufferList.cpp
index 68a1e80..724c7dd 100644
--- a/src/BufferList.cpp
+++ b/src/BufferList.cpp
@@ -130,9 +130,9 @@ Buffer * BufferList::newBuffer(string const & s)
 
 Buffer * BufferList::createNewBuffer(string const & s)
 {
-	auto_ptr tmpbuf;
+	Buffer * tmpbuf;
 	try {
-		tmpbuf.reset(new Buffer(s));
+		tmpbuf = new Buffer(s);
 	} catch (ExceptionMessage const & message) {
 		if (message.type_ == ErrorException) {
 			Alert::error(message.title_, message.details_);
@@ -143,7 +143,7 @@ Buffer * BufferList::createNewBuffer(string const & s)
 		}
 	}
 	tmpbuf->params().useClassDefaults();
-	return tmpbuf.release();
+	return tmpbuf;
 }
 
 

Re: C++ question about auto_ptr

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> With C++11, auto_ptr is deprecated and we get warnings.
> 
> I am trying to see how we can get rid of it. unique_ptr is new to C++11,
> so I'd rather avoid that.

Why not use unique_ptr fpr C++11 and auto_ptr else? The difference is in the 
copying semantics, which does not matter for the current use cases. The only 
challenge would be to find a good name fo the typedef.

> I have many places like in the patch below. Is there a reason why I
> should keep the auto_ptr instead of a naked pointer? What is it good for?

Usually it is used for exception safety: If you use a raw pointer, you need 
to delete it in the catch clause, else you get a memory leak. This would be 
easy to do in this particular example, but impossible if the catch happens 
somewhere completely different.


Georg

Wrong enum as boolean usage in Text3.cpp

[Stephan Witt]

Since change 02cb5fd3 one can see the clever code fragment in Text3.cpp line 
2934..2943 has a problem with Clipboard::PdfGraphicsType. The assignment of 
type for "pdf" at line 2935 is not true for arg value "pdf". The enum 
Clipboard::GraphicsType shouldn't start with PdfGraphicsType or the code block 
has to be changed. Solution 1 is attached as patch. Ok to apply?

Stephan


PdfGraphicsType.patch
Description: Binary data

Re: Wrong enum as boolean usage in Text3.cpp

[Scott Kostyshak]

On Fri, May 8, 2015 at 10:29 PM, Stephan Witt  wrote:
> Since change 02cb5fd3 one can see the clever code fragment in Text3.cpp line 
> 2934..2943 has a problem with Clipboard::PdfGraphicsType. The assignment of 
> type for "pdf" at line 2935 is not true for arg value "pdf". The enum 
> Clipboard::GraphicsType shouldn't start with PdfGraphicsType or the code 
> block has to be changed. Solution 1 is attached as patch. Ok to apply?

I was just working on the same thing. I imagine you looked into it
because of the message on the terminal also? In any case, my patch is
attached.

I do not like the approach you propose because if the code is changed
around we could see the same problem. I don't have a realistic idea in
mind for that though. Actually, it's not that I don't like the
approach you took, I don't like that code style for the if/if-else. It
just seems like going a little too much out of the way to save
vertical space. In some sense, I admit that your patch is the least
invasive because it is a small patch.

Does that code style lead to an optimization somehow?

I don't have a strong opinion on this. However, if you like your patch
better than the one I propose, would it be OK to define the top one to
be a number greater than 0?

Scott
From c7ce0bb2e1ffbc5b492d800aca82181d33f03cfa Mon Sep 17 00:00:00 2001
From: Scott Kostyshak 
Date: Fri, 8 May 2015 19:31:47 -0400
Subject: [PATCH] Fix pasting of PDF from clipboard

The command "paste pdf" was always disabled because the
condition in the following "if" statement always returns false

  if (arg == "pdf" && (type = Clipboard::PdfGraphicsType))

The value of "type" is zero in this case because PdfGraphicsType is
the first enum value (and it is not set explicitly to non-zero).

An alternative patch is to put AnyGraphicsType as the first
element of enum GraphicsType, or to set the first element to a
number greater than 0.

To test the bug that this commit fixes, either copy a PDF and try to
paste with the action "paste pdf", or click on the "Edit" menu and
notice (before this commit) the terminal output "Unrecognized
graphics type: pdf".
---
 src/Text3.cpp | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/Text3.cpp b/src/Text3.cpp
index f80bde6..dbff72a 100644
--- a/src/Text3.cpp
+++ b/src/Text3.cpp
@@ -2930,25 +2930,30 @@ bool Text::getStatus(Cursor & cur, FuncRequest const & cmd,
 			break;
 		}
 
-		// explicit graphics type?
 		Clipboard::GraphicsType type = Clipboard::AnyGraphicsType;
-		if ((arg == "pdf" && (type = Clipboard::PdfGraphicsType))
-			  || (arg == "png" && (type = Clipboard::PngGraphicsType))
-			  || (arg == "jpeg" && (type = Clipboard::JpegGraphicsType))
-			  || (arg == "linkback" &&  (type = Clipboard::LinkBackGraphicsType))
-			  || (arg == "emf" &&  (type = Clipboard::EmfGraphicsType))
-			  || (arg == "wmf" &&  (type = Clipboard::WmfGraphicsType))) {
-			enable = theClipboard().hasGraphicsContents(type);
+		if (arg == "pdf")
+			type = Clipboard::PdfGraphicsType;
+		else if (arg == "png")
+			type = Clipboard::PngGraphicsType;
+		else if (arg == "jpeg")
+			type = Clipboard::JpegGraphicsType;
+		else if (arg == "linkback")
+			type = Clipboard::LinkBackGraphicsType;
+		else if (arg == "emf")
+			type = Clipboard::EmfGraphicsType;
+		else if (arg == "wmf")
+			type = Clipboard::WmfGraphicsType;
+		else {
+			// unknown argument
+			LYXERR0("Unrecognized graphics type: " << arg);
+			// we don't want to assert if the user just mistyped the LFUN
+			LATTEST(cmd.origin() != FuncRequest::INTERNAL);
+			enable = false;
 			break;
 		}
-
-		// unknown argument
-		LYXERR0("Unrecognized graphics type: " << arg);
-		// we don't want to assert if the user just mistyped the LFUN
-		LATTEST(cmd.origin() != FuncRequest::INTERNAL);
-		enable = false;
+		enable = theClipboard().hasGraphicsContents(type);
 		break;
-	 }
+	}
 
 	case LFUN_CLIPBOARD_PASTE:
 	case LFUN_CLIPBOARD_PASTE_SIMPLE:
-- 
2.1.4

Re: Wrong enum as boolean usage in Text3.cpp

[Stephan Witt]

Am 09.05.2015 um 05:12 schrieb Scott Kostyshak :

> On Fri, May 8, 2015 at 10:29 PM, Stephan Witt  wrote:
>> Since change 02cb5fd3 one can see the clever code fragment in Text3.cpp line 
>> 2934..2943 has a problem with Clipboard::PdfGraphicsType. The assignment of 
>> type for "pdf" at line 2935 is not true for arg value "pdf". The enum 
>> Clipboard::GraphicsType shouldn't start with PdfGraphicsType or the code 
>> block has to be changed. Solution 1 is attached as patch. Ok to apply?
> 
> I was just working on the same thing. I imagine you looked into it
> because of the message on the terminal also? In any case, my patch is
> attached.

This one is obviously more readable and you should apply your patch.

> I do not like the approach you propose because if the code is changed
> around we could see the same problem. I don't have a realistic idea in
> mind for that though. Actually, it's not that I don't like the
> approach you took, I don't like that code style for the if/if-else. It
> just seems like going a little too much out of the way to save
> vertical space. In some sense, I admit that your patch is the least
> invasive because it is a small patch.
> 
> Does that code style lead to an optimization somehow?
> 
> I don't have a strong opinion on this. However, if you like your patch
> better than the one I propose, would it be OK to define the top one to
> be a number greater than 0?

This solution I've tried too and it would work. But the next enum in 
Clipboard.h (TextType) starts with an Any type too and so I made them more 
similar. I prefer this for enums like that because extending them doesn't 
change the existing values. Of course this is not really important.

Stephan