Re: [patch] My first LFUN

2015-12-20 Thread Guillaume Munch 

Le 20/12/2015 22:42, Jean-Marc Lasgouttes a écrit :

Le 17/12/2015 01:36, Guillaume Munch a écrit :

Here's a version with tabular-feature instead of table-modify (and no
change to Shortcuts.lyx given that the docs are now frozen).


A few comments on patches (not all of them for you):
* you still handle inset-modify tabular for documents and of binding
files, right?


I am not sure to understand the question. "Inset-modify tabular" still
exists for the tabular dialog. It does not make much sense to use it for
binding files and documentation for the commands used currently.


Richard, how difficult would it be to use lf2lfun in
lyx2lyx to translate info insets? We do not really know what version of
lfuns we have in lyx files, do we?


I planned to change everything with S when the docs are returned 
before release. Do you think we need a permanent conversion?




* does the lfun2lfun update require a new version for our ui/bind files?

* did you update LFUNS.lyx using the relevant script (I guess you did,
but want to be sure).


Yes



* in the 3rd patch, why don't you return "false" instead of
  case LFUN_TABULAR_FEATURE: {
+if (!isTable()) {
+status.clear();
+status.setUnknown(true);
+return true;
+}


If !isTable(), then we want to ask the enclosing tabular, if exists. Is
replacing the three nested lines with "return false" going to achieve
the same?

Thanks.

Re: [patch] My first LFUN

2015-12-20 Thread Jean-Marc Lasgouttes 

Le 17/12/2015 01:36, Guillaume Munch a écrit :

Here's a version with tabular-feature instead of table-modify (and no
change to Shortcuts.lyx given that the docs are now frozen).


A few comments on patches (not all of them for you):
* you still handle inset-modify tabular for documents and of binding 
files, right? Richard, how difficult would it be to use lf2lfun in 
lyx2lyx to translate info insets? We do not really know what version of 
lfuns we have in lyx files, do we?


* does the lfun2lfun update require a new version for our ui/bind files?

* did you update LFUNS.lyx using the relevant script (I guess you did, 
but want to be sure).


* in the 3rd patch, why don't you return "false" instead of
case LFUN_TABULAR_FEATURE: {
+   if (!isTable()) {
+   status.clear();
+   status.setUnknown(true);
+   return true;
+   }


Note that I only read the poatches, I did not try them out yet.

JMarc

Re: [patch] My first LFUN

2015-12-20 Thread Richard Heck 
On 12/20/2015 06:12 PM, Guillaume Munch wrote:
> Le 20/12/2015 22:42, Jean-Marc Lasgouttes a écrit :
>> Le 17/12/2015 01:36, Guillaume Munch a écrit :
>>> Here's a version with tabular-feature instead of table-modify (and no
>>> change to Shortcuts.lyx given that the docs are now frozen).
>>
>> A few comments on patches (not all of them for you):
>> * you still handle inset-modify tabular for documents and of binding
>> files, right?
>
> I am not sure to understand the question. "Inset-modify tabular" still
> exists for the tabular dialog. It does not make much sense to use it for
> binding files and documentation for the commands used currently.

The problem is that people may have customized bind and ui files that use
"inset-modify tabular". JMarc's question is: Will those commands still work
after your patch?

>> * does the lfun2lfun update require a new version for our ui/bind files?

The answer to this question thus partly depends upon the answer to the
former
one. If the old "inset-modify tabular" commands no longer work, then we
need a
format change of the ui and bind files. Indeed, one might think we
should have a
format change anyway, since the old syntax might now be regarded as
deprecated.

>> Richard, how difficult would it be to use lf2lfun in lyx2lyx to
>> translate
>> info insets? We do not really know what version of lfuns we have in lyx
>> files, do we?
>
> I planned to change everything with S when the docs are returned before
> release. Do you think we need a permanent conversion?

Note that this is now kind of the same question, but concerning the
docs. In this
case, I'm inclined to think that what we need is a format change of the
LyX files.
(In answer to JMarc: Since there's no mapping from LyX formats to LFUN
formats,
we can't use prefs2prefs to fix this.)

If that seems weird: Sometimes format changes aren't really format changes.
Rather, they are like hooks that force lyx2lyx to be run. In this case,
the plan
would be to search for uses of "inset-modify tabular" within InsetInfo and
replace them with the corresponding case of "tabular-modify". Here
again, the
problem is that someone might have used "inset-modify tabular" in their own
files, with InsetInfo. Think e.g. of someone who might have made a set
of LyX
teaching manuals for use in their own group.

Richard

Re: [patch] My first LFUN

2015-12-20 Thread Guillaume Munch 

Le 20/12/2015 19:28, Jean-Marc Lasgouttes a écrit :

Le 20/12/15 19:49, Guillaume Munch a écrit :

Here's a version with tabular-feature instead of table-modify (and no
change to Shortcuts.lyx given that the docs are now frozen).


It would be nice if somebody could review this before the feature freeze.


I'll try to have a go tomorrow. I've been busy these days.



Thanks

Re: [patch] My first LFUN

2015-12-20 Thread Guillaume Munch 

Le 17/12/2015 00:36, Guillaume Munch a écrit :

Le 16/12/2015 13:30, Jean-Marc Lasgouttes a écrit :

Le 11/12/2015 23:36, Guillaume Munch a écrit :

Dear list,


Following the discussions in , here
are a series of patches meant to address #9794 (inset-modify tabular
commands are incorrectly disabled) and #4189 (Edit->Rows not
shown when in mathed insert).


Dear Guillaume,

I do not have time right now to look at the patch, but I do not like the
'tricks' to avoid renaming icons. The easiest way out would probably be
to rename to the original tabular-features LFUN name.



Here's a version with tabular-feature instead of table-modify (and no
change to Shortcuts.lyx given that the docs are now frozen).




It would be nice if somebody could review this before the feature freeze.


Guillaume

Re: [patch] My first LFUN

2015-12-20 Thread Richard Heck 
On 12/16/2015 07:36 PM, Guillaume Munch wrote:
> Le 16/12/2015 13:30, Jean-Marc Lasgouttes a écrit :
>> Le 11/12/2015 23:36, Guillaume Munch a écrit :
>>> Dear list,
>>>
>>>
>>> Following the discussions in ,
>>> here
>>> are a series of patches meant to address #9794 (inset-modify tabular
>>> commands are incorrectly disabled) and #4189 (Edit->Rows not
>>> shown when in mathed insert).
>>
>> Dear Guillaume,
>>
>> I do not have time right now to look at the patch, but I do not like the
>> 'tricks' to avoid renaming icons. The easiest way out would probably be
>> to rename to the original tabular-features LFUN name.
>>
>
> Here's a version with tabular-feature instead of table-modify (and no
> change to Shortcuts.lyx given that the docs are now frozen).

I try not to mess with tabular stuff, since I almost never use tables.

Richard

Re: [patch] My first LFUN

2015-12-20 Thread Jean-Marc Lasgouttes 

Le 20/12/15 19:49, Guillaume Munch a écrit :

Here's a version with tabular-feature instead of table-modify (and no
change to Shortcuts.lyx given that the docs are now frozen).


It would be nice if somebody could review this before the feature freeze.


I'll try to have a go tomorrow. I've been busy these days.

JMarc

Re: [patch] My first LFUN

2015-12-16 Thread Guillaume Munch 

Le 16/12/2015 13:30, Jean-Marc Lasgouttes a écrit :

Le 11/12/2015 23:36, Guillaume Munch a écrit :

Dear list,


Following the discussions in , here
are a series of patches meant to address #9794 (inset-modify tabular
commands are incorrectly disabled) and #4189 (Edit->Rows not
shown when in mathed insert).


Dear Guillaume,

I do not have time right now to look at the patch, but I do not like the
'tricks' to avoid renaming icons. The easiest way out would probably be
to rename to the original tabular-features LFUN name.



Here's a version with tabular-feature instead of table-modify (and no 
change to Shortcuts.lyx given that the docs are now frozen).


Guillaume
>From 4d6feb4354bff0067d82ba82cd71f3b30df11862 Mon Sep 17 00:00:00 2001
From: Guillaume Munch 
Date: Fri, 11 Dec 2015 02:15:52 +
Subject: [PATCH 1/3] New LFUN tabular-features (#9794)

The tabular-features LFUN was merged with "inset-modify tabular" when
simplifying the tabular dialog at b5049e7. This choice later indirectly caused a
few regressions (#7308, #9794).

I reintroduce tabular-feature to allow more flexibility for user
commands, whereas "inset-modify tabular" is now reserved for the tabular
dialog. In particular, inset-modify tabular is no longer caught by math grid
insets. The name tabular-feature is kept to avoid renaming icons.

Known issues:

* After successfully applying a tabular command, the cursor is truncated to the
  table.

* Note that the tabular dialog still has similar issues that are inherited from
  the achitecture of the dialog menu. For instance the pref change can be
  mis-dispatched to an inset inside a cell and cause an error, for instance:

Lexer.cpp (934): Missing 'Note'-tag in InsetNote::string2params. Got
tabular instead. Line: 0

  Maybe the inset-modify LFUN should be modified to treat commands coming from
  the wrong dialog (by checking the type) as unknown and undispatched so that
  the parent can get it. In that case a non AtPoint variant of inset-modify
  could be reintroduced in order to generalise tabular-feature.
---
 src/FuncCode.h   |  1 +
 src/LyXAction.cpp| 25 -
 src/frontends/qt4/GuiApplication.cpp | 11 +++---
 src/frontends/qt4/GuiTabular.cpp |  7 ++--
 src/insets/Inset.cpp |  2 --
 src/insets/InsetTabular.cpp  | 70 +---
 src/insets/InsetTabular.h|  3 ++
 src/mathed/InsetMathAMSArray.cpp |  9 ++---
 src/mathed/InsetMathCases.cpp| 18 +++---
 src/mathed/InsetMathGrid.cpp | 27 +-
 src/mathed/InsetMathHull.cpp |  9 ++---
 src/mathed/InsetMathSplit.cpp| 12 +++
 src/mathed/InsetMathSubstack.cpp | 16 +++--
 13 files changed, 95 insertions(+), 115 deletions(-)

diff --git a/src/FuncCode.h b/src/FuncCode.h
index 66f57c0..40ec9d3 100644
--- a/src/FuncCode.h
+++ b/src/FuncCode.h
@@ -464,6 +464,7 @@ enum FuncCode
 	LFUN_BUFFER_MOVE_NEXT,  // skostysh 20150408
 	// 340
 	LFUN_BUFFER_MOVE_PREVIOUS,  // skostysh 20150408
+	LFUN_TABULAR_FEATURE,   // gm, 20151210
 	LFUN_LASTACTION // end of the table
 };
 
diff --git a/src/LyXAction.cpp b/src/LyXAction.cpp
index 4467219..1e83905 100644
--- a/src/LyXAction.cpp
+++ b/src/LyXAction.cpp
@@ -2167,7 +2167,7 @@ void LyXAction::init()
 /*!
  * \var lyx::FuncCode lyx::LFUN_TABULAR_INSERT
  * \li Action: Inserts table into the document.
- * \li Notion: See #LFUN_INSET_MODIFY for some more details
+ * \li Notion: See #LFUN_TABULAR_FEATURE for some more details
about tabular modifications.
  * \li Syntax: tabular-insert [ ]
  * \li Params: In case no arguments are given show insert dialog.
@@ -2462,11 +2462,17 @@ void LyXAction::init()
 ref, space, tabular, vspace, wrap insets.
  * \li Syntax: inset-modify  
  * \li Syntax: inset-modify changetype 
- * \li Syntax: inset-modify tabular  []
+ * \li Sample: inset-modify note Note Comment \n
+	   inset-modify changetype Ovalbox
+ * \endvar
+ */
+		{ LFUN_INSET_MODIFY, "inset-modify", AtPoint, Edit },
+/*!
+ * \var lyx::FuncCode lyx::LFUN_TABULAR_FEATURE
+ * \li Action: Modify properties of tabulars and table-like math environments.
+ * \li Syntax: tabular-feature  []
  * \li Params: Generally see #LFUN_INSET_INSERT for further details.\n
-   In case that  is "tabular" various math-environment features
-   are handled as well, e.g. add-vline-left/right for the Grid/Array environment.\n
-   : append-row|append-column|delete-row|delete-column|copy-row|\n
+ * : append-row|append-column|delete-row|delete-column|copy-row|\n
copy-column|move-column-right|move-column-left|move-row-down|move-row-up|\n
toggle-line-top|toggle-line-bottom|toggle-line-left|toggle-line-right|\n

Re: [patch] My first LFUN

2015-12-16 Thread Jean-Marc Lasgouttes 
Le 11/12/2015 23:36, Guillaume Munch a écrit :
> Dear list,
> 
> 
> Following the discussions in , here
> are a series of patches meant to address #9794 (inset-modify tabular
> commands are incorrectly disabled) and #4189 (Edit->Rows not
> shown when in mathed insert).

Dear Guillaume,

I do not have time right now to look at the patch, but I do not like the
'tricks' to avoid renaming icons. The easiest way out would probably be
to rename to the original tabular-features LFUN name.

JMarc

Re: [patch] My first LFUN

2015-12-16 Thread Guillaume Munch 

Le 16/12/2015 13:30, Jean-Marc Lasgouttes a écrit :

Le 11/12/2015 23:36, Guillaume Munch a écrit :

Dear list,


Following the discussions in , here
are a series of patches meant to address #9794 (inset-modify tabular
commands are incorrectly disabled) and #4189 (Edit->Rows not
shown when in mathed insert).


Dear Guillaume,

I do not have time right now to look at the patch, but I do not like the
'tricks' to avoid renaming icons. The easiest way out would probably be
to rename to the original tabular-features LFUN name.



These tricks were already there since the patch that removed the 
tabular-feature lfun. I am not too fond of this either, but there is no 
regression on this aspect. I could reuse the tabular-feature name, but 
please note that removing the mapping completely would require to 
(automatically) change all the icons names in the manual. Were you 
speaking of also removing the mapping that existed previously?

Re: [patch] My first LFUN

2015-12-16 Thread Guillaume Munch 

Le 15/12/2015 18:03, Richard Heck a écrit :
On 12/11/2015 05:36 PM, Guillaume Munch wrote:  >> Dear list, >> >> >> Following the discussions in 
, >> here are a series of patches 
meant to address #9794 (inset-modify >> tabular commands are incorrectly 
disabled) and #4189 >> (Edit->Rows not shown when in mathed 
insert). > > There is a somewhat more general issue here: Dispatched 
LFUNs are > generally caught by the first inset that knows how to handle 
the > relevant command. So this can cause problems with embedded insets. 
I > did some work on this a while ago in the InsetModify-Incomplete > 
branch of my personal repo: > 
http://git.lyx.org/?p=developers/rgheck/lyx.git;a=summary I'm not > sure 
how this relates to your patches. > > Richard >


Dear Richard

Please make sure that you reply to the list :)

Regarding 2.2.0:

It is clear that the change you are suggesting is bigger and requires 
more testing. This does not look like something that can be done for 
2.2.0. Thus my question is, how do we want to address #9794 for the next 
two years? Because in any case, we need some new lfun which is not 
AtPoint. Indeed, for tabular commands, it is meaningless to dispatch to 
the inset in front of the cursor because we cannot guess which cell is 
targeted.


This new lfun table-modify was the best option as far as the discussions 
went in the ticket, and it does not preclude a bigger change for 2.3.0. 
Therefore, can I please commit this patch?


Regarding 2.3.0:

You are right, while writing the patch I indeed thought that a bigger 
problem was maybe that insets should not catch inset-modify commands not 
meant for them. Similar issues remain in the tabular dialog for this 
reason, as I wrote in the commit. I now think that one should, as you do 
in your patch above, test for the type of inset passed to inset-modify 
and leave the lfun undispatched when it is not the good one. This avoids 
the problem mentioned by Jean-Marc in the above ticket regarding 
"hardcoding knowledge of other insets everywhere". This would also 
prevent issues like , so much that 
I wonder why this was not done already.


Sincerely
Guillaume

[patch] My first LFUN

2015-12-11 Thread Guillaume Munch 

Dear list,


Following the discussions in , here 
are a series of patches meant to address #9794 (inset-modify tabular 
commands are incorrectly disabled) and #4189 (Edit->Rows not 
shown when in mathed insert).


What I still don't like is that the cursor is truncated after applying 
table commands (issue mentioned in the first patch). If somebody can 
point me to an example where one does restore the full cursor after a 
dispatch, or tell me how to do that, then I will be able to fix this as 
well in the near future.


Uwe, do not worry about the changes in the docs. Eventually, only 
Shortcut.lyx has to be changed, and if the patch is not pushed in time 
this can be done later in the translations, automatically.




Sincerely,
Guillaume
>From d490d7d4693c3cd2198e3dd96904f19b47f2b3be Mon Sep 17 00:00:00 2001
From: Guillaume Munch 
Date: Fri, 11 Dec 2015 02:15:52 +
Subject: [PATCH 1/3] New LFUN table-modify (#9794)

The tabular-features LFUN was merged with "inset-modify tabular" when
simplifying the tabular dialog at b5049e7. This choice later indirectly caused a
few regressions (#7308, #9794).

I introduce a separate LFUN table-modify to allow more flexibility for user
commands, whereas "inset-modify tabular" is now reserved for the tabular
dialog. In particular, inset-modify tabular is no longer caught by math grid
insets.

Known issues:

* After successfully applying a tabular command, the cursor is truncated to the
  table.

* Note that the tabular dialog still has similar issues that are inherited from
  the achitecture of the dialog menu. For instance the pref change can be
  mis-dispatched to an inset inside a cell and cause an error, for instance:
  Lexer.cpp (934): Missing 'Note'-tag in InsetNote::string2params. Got
  tabular instead. Line: 0
  Maybe the inset-modify LFUN should treat commands coming from the wrong
  dialog as unknown and undispatched so that the parent can get it.
---
 src/FuncCode.h   |  1 +
 src/LyXAction.cpp| 25 -
 src/frontends/qt4/GuiApplication.cpp | 17 +
 src/frontends/qt4/GuiTabular.cpp |  7 ++--
 src/insets/Inset.cpp |  2 --
 src/insets/InsetTabular.cpp  | 70 +---
 src/insets/InsetTabular.h|  3 ++
 src/mathed/InsetMathAMSArray.cpp |  9 ++---
 src/mathed/InsetMathCases.cpp| 20 +++
 src/mathed/InsetMathGrid.cpp | 27 +-
 src/mathed/InsetMathHull.cpp |  9 ++---
 src/mathed/InsetMathSplit.cpp| 12 +++
 src/mathed/InsetMathSubstack.cpp | 16 +++--
 13 files changed, 101 insertions(+), 117 deletions(-)

diff --git a/src/FuncCode.h b/src/FuncCode.h
index 66f57c0..e8a524b 100644
--- a/src/FuncCode.h
+++ b/src/FuncCode.h
@@ -464,6 +464,7 @@ enum FuncCode
 	LFUN_BUFFER_MOVE_NEXT,  // skostysh 20150408
 	// 340
 	LFUN_BUFFER_MOVE_PREVIOUS,  // skostysh 20150408
+	LFUN_TABLE_MODIFY,  // gm, 20151210
 	LFUN_LASTACTION // end of the table
 };
 
diff --git a/src/LyXAction.cpp b/src/LyXAction.cpp
index 4467219..b4cb5f4 100644
--- a/src/LyXAction.cpp
+++ b/src/LyXAction.cpp
@@ -2167,7 +2167,7 @@ void LyXAction::init()
 /*!
  * \var lyx::FuncCode lyx::LFUN_TABULAR_INSERT
  * \li Action: Inserts table into the document.
- * \li Notion: See #LFUN_INSET_MODIFY for some more details
+ * \li Notion: See #LFUN_TABLE_MODIFY for some more details
about tabular modifications.
  * \li Syntax: tabular-insert [ ]
  * \li Params: In case no arguments are given show insert dialog.
@@ -2462,11 +2462,17 @@ void LyXAction::init()
 ref, space, tabular, vspace, wrap insets.
  * \li Syntax: inset-modify  
  * \li Syntax: inset-modify changetype 
- * \li Syntax: inset-modify tabular  []
+ * \li Sample: inset-modify note Note Comment \n
+	   inset-modify changetype Ovalbox
+ * \endvar
+ */
+		{ LFUN_INSET_MODIFY, "inset-modify", AtPoint, Edit },
+/*!
+ * \var lyx::FuncCode lyx::LFUN_TABLE_MODIFY
+ * \li Action: Modify properties of tabulars and table-like math environments.
+ * \li Syntax: table-modify  []
  * \li Params: Generally see #LFUN_INSET_INSERT for further details.\n
-   In case that  is "tabular" various math-environment features
-   are handled as well, e.g. add-vline-left/right for the Grid/Array environment.\n
-   : append-row|append-column|delete-row|delete-column|copy-row|\n
+ * : append-row|append-column|delete-row|delete-column|copy-row|\n
copy-column|move-column-right|move-column-left|move-row-down|move-row-up|\n
toggle-line-top|toggle-line-bottom|toggle-line-left|toggle-line-right|\n
align-left|align-right|align-center|align-block|align-decimal|set-decimal-point|\n
@@ -2481,13 +2487,14 @@ void LyXAction::init()