Re: r37704 - lyx-devel/trunk/src/insets
On Thu, Feb 17, 2011 at 11:22 AM, Enrico Forestieri wrote: > On Thu, Feb 17, 2011 at 09:33:44AM +0100, Edwin Leuven wrote: > > as discussed in another thread, the problem was that only the 1st > > argument got checked, and that was just an arbitrary one (the one > > which just happened to be added first in the frontend code). which is > > prone to errors as we saw. so it seems to me that vincent's fix is the > > correct one. > > Well, even if vincent's fix is correct, he himself is not correct. > > Q: Why are you reverting it? > A: Because I'm right and you're wrong. > > End of discussion, begin to give vent to one's anger. > Only later I noticed the other thread. > I am sure Vincent did not meant to hurt you with his revert. He explained in the commit log why he was reverting this. He probably should have warned you that he was going to, but this is not like there is a lot of disrespect. I am actually pretty sure you he respects you. Anyway, that was just my monthly two cents :-P Abdel.
Re: r37704 - lyx-devel/trunk/src/insets
On Thu, Feb 17, 2011 at 09:33:44AM +0100, Edwin Leuven wrote: > as discussed in another thread, the problem was that only the 1st > argument got checked, and that was just an arbitrary one (the one > which just happened to be added first in the frontend code). which is > prone to errors as we saw. so it seems to me that vincent's fix is the > correct one. Well, even if vincent's fix is correct, he himself is not correct. Q: Why are you reverting it? A: Because I'm right and you're wrong. End of discussion, begin to give vent to one's anger. Only later I noticed the other thread. -- Enrico
Re: r37704 - lyx-devel/trunk/src/insets
On Thu, Feb 17, 2011 at 01:26, Enrico Forestieri wrote: > On Thu, Feb 17, 2011 at 01:09:04AM +0100, Pavel Sanda wrote: >> Vincent van Ravesteijn wrote: >> >> Which has no effect at all. >> >> >> > It does. >> >> can both of you please come back to this point and in more than one sentence >> put down your arguments/analysis? > > I am sorry, this is a behavioural problem. I would not have any problem > discussing and even acknowledging that I could be wrong, but > there are ways to do things and the presuntuous way I cannot stand. hehe, why do things always become fun after i go to bed? as vincent wrote, with your patch you can "execute "inset-modify tabular set-tabular-width 7cm" for a longtable, and it happily changes the table, while this lfun should have been disabled." i also see this, and it is because the function is enabled whenever tabular.tabular_width.zero() and therefore also for longtables etc as discussed in another thread, the problem was that only the 1st argument got checked, and that was just an arbitrary one (the one which just happened to be added first in the frontend code). which is prone to errors as we saw. so it seems to me that vincent's fix is the correct one. cheers, ed.
Re: r37704 - lyx-devel/trunk/src/insets
On Thu, Feb 17, 2011 at 01:09:04AM +0100, Pavel Sanda wrote: > Vincent van Ravesteijn wrote: > >> Which has no effect at all. > >> > > It does. > > can both of you please come back to this point and in more than one sentence > put down your arguments/analysis? I am sorry, this is a behavioural problem. I would not have any problem discussing and even acknowledging that I could be wrong, but there are ways to do things and the presuntuous way I cannot stand. > and if possible no reverts while flaming, otherwise we might have correct code > but two developers left... I am not a developer, so you will lose nothing. -- Enrico
Re: r37704 - lyx-devel/trunk/src/insets
Vincent van Ravesteijn wrote: >> Which has no effect at all. >> > It does. can both of you please come back to this point and in more than one sentence put down your arguments/analysis? and if possible no reverts while flaming, otherwise we might have correct code but two developers left... thanks, pavel
Re: r37704 - lyx-devel/trunk/src/insets
On Thu, Feb 17, 2011 at 12:29:18AM +0100, Vincent van Ravesteijn wrote: > > This is basically saying that SET_TABULAR_WIDTH should be allowed > for longtables, because tabular_width is zero by definition for > longtables. > >>>It works just fine. > >>> > >>It does not. It allows you to execute "inset-modify tabular > >>set-tabular-width 7cm" for a longtable, and it happily changes the > >>table, while this lfun should have been disabled. > >Which has no effect at all. > > > It does. Good bye, I have better things to do. -- Enrico
Re: r37704 - lyx-devel/trunk/src/insets
This is basically saying that SET_TABULAR_WIDTH should be allowed for longtables, because tabular_width is zero by definition for longtables. It works just fine. It does not. It allows you to execute "inset-modify tabular set-tabular-width 7cm" for a longtable, and it happily changes the table, while this lfun should have been disabled. Which has no effect at all. It does. Vincent
Re: r37704 - lyx-devel/trunk/src/insets
On Thu, Feb 17, 2011 at 12:20:49AM +0100, Vincent van Ravesteijn wrote: > Op 17-2-2011 0:17, Enrico Forestieri schreef: > >On Wed, Feb 16, 2011 at 11:57:22PM +0100, Vincent van Ravesteijn wrote: > >> Op 16-2-2011 23:19, for...@lyx.org schreef: > >>>Author: forenr > >>>Date: Wed Feb 16 23:19:49 2011 > >>>New Revision: 37704 > >>>URL: http://www.lyx.org/trac/changeset/37704 > >>> > >>>Log: > >>>Don't disable apply button if one (or more) of vertical alignment, > >>>rotation, > >>>or long table settings are changed. Fixes bug #7308. > >>> > >>>Modified: > >>>lyx-devel/trunk/src/insets/InsetTabular.cpp > >>> > >>>Modified: lyx-devel/trunk/src/insets/InsetTabular.cpp > >>>== > >>>--- lyx-devel/trunk/src/insets/InsetTabular.cppWed Feb 16 22:08:18 > >>>2011(r37703) > >>>+++ lyx-devel/trunk/src/insets/InsetTabular.cppWed Feb 16 23:19:49 > >>>2011(r37704) > >>>@@ -4323,8 +4323,10 @@ > >>> return true; > >>> > >>> case Tabular::SET_TABULAR_WIDTH: > >>>- status.setEnabled(!tabular.rotate&& > >>>!tabular.is_long_tabular > >>>- && tabular.tabular_valignment == > >>>Tabular::LYX_VALIGN_MIDDLE); > >>>+ status.setEnabled(tabular.tabular_width.zero() > >>>+ || (!tabular.rotate&& > >>>!tabular.is_long_tabular > >>>+ && tabular.tabular_valignment == > >>>+ Tabular::LYX_VALIGN_MIDDLE)); > >>> break; > >>> > >>> case Tabular::SET_DECIMAL_POINT: > >>This is not correct. > >I'm sorry, you're wrong. > > > I'm sorry, I'm right. You are only a conceited young man. > >>This is basically saying that SET_TABULAR_WIDTH should be allowed > >>for longtables, because tabular_width is zero by definition for > >>longtables. > >It works just fine. > > > It does not. It allows you to execute "inset-modify tabular > set-tabular-width 7cm" for a longtable, and it happily changes the > table, while this lfun should have been disabled. Which has no effect at all. -- Enrico
Re: r37704 - lyx-devel/trunk/src/insets
Op 17-2-2011 0:17, Enrico Forestieri schreef: On Wed, Feb 16, 2011 at 11:57:22PM +0100, Vincent van Ravesteijn wrote: Op 16-2-2011 23:19, for...@lyx.org schreef: Author: forenr Date: Wed Feb 16 23:19:49 2011 New Revision: 37704 URL: http://www.lyx.org/trac/changeset/37704 Log: Don't disable apply button if one (or more) of vertical alignment, rotation, or long table settings are changed. Fixes bug #7308. Modified: lyx-devel/trunk/src/insets/InsetTabular.cpp Modified: lyx-devel/trunk/src/insets/InsetTabular.cpp == --- lyx-devel/trunk/src/insets/InsetTabular.cpp Wed Feb 16 22:08:18 2011 (r37703) +++ lyx-devel/trunk/src/insets/InsetTabular.cpp Wed Feb 16 23:19:49 2011 (r37704) @@ -4323,8 +4323,10 @@ return true; case Tabular::SET_TABULAR_WIDTH: - status.setEnabled(!tabular.rotate&& !tabular.is_long_tabular - && tabular.tabular_valignment == Tabular::LYX_VALIGN_MIDDLE); + status.setEnabled(tabular.tabular_width.zero() + || (!tabular.rotate&& !tabular.is_long_tabular + && tabular.tabular_valignment == + Tabular::LYX_VALIGN_MIDDLE)); break; case Tabular::SET_DECIMAL_POINT: This is not correct. I'm sorry, you're wrong. I'm sorry, I'm right. This is basically saying that SET_TABULAR_WIDTH should be allowed for longtables, because tabular_width is zero by definition for longtables. It works just fine. It does not. It allows you to execute "inset-modify tabular set-tabular-width 7cm" for a longtable, and it happily changes the table, while this lfun should have been disabled. Vincent
Re: r37704 - lyx-devel/trunk/src/insets
On Wed, Feb 16, 2011 at 11:57:22PM +0100, Vincent van Ravesteijn wrote: > Op 16-2-2011 23:19, for...@lyx.org schreef: > >Author: forenr > >Date: Wed Feb 16 23:19:49 2011 > >New Revision: 37704 > >URL: http://www.lyx.org/trac/changeset/37704 > > > >Log: > >Don't disable apply button if one (or more) of vertical alignment, rotation, > >or long table settings are changed. Fixes bug #7308. > > > >Modified: > >lyx-devel/trunk/src/insets/InsetTabular.cpp > > > >Modified: lyx-devel/trunk/src/insets/InsetTabular.cpp > >== > >--- lyx-devel/trunk/src/insets/InsetTabular.cpp Wed Feb 16 22:08:18 > >2011(r37703) > >+++ lyx-devel/trunk/src/insets/InsetTabular.cpp Wed Feb 16 23:19:49 > >2011(r37704) > >@@ -4323,8 +4323,10 @@ > > return true; > > > > case Tabular::SET_TABULAR_WIDTH: > >-status.setEnabled(!tabular.rotate&& > >!tabular.is_long_tabular > >-&& tabular.tabular_valignment == > >Tabular::LYX_VALIGN_MIDDLE); > >+status.setEnabled(tabular.tabular_width.zero() > >+|| (!tabular.rotate&& !tabular.is_long_tabular > >+&& tabular.tabular_valignment == > >+Tabular::LYX_VALIGN_MIDDLE)); > > break; > > > > case Tabular::SET_DECIMAL_POINT: > > This is not correct. I'm sorry, you're wrong. > This is basically saying that SET_TABULAR_WIDTH should be allowed > for longtables, because tabular_width is zero by definition for > longtables. It works just fine. -- Enrico
Re: r37704 - lyx-devel/trunk/src/insets
Op 16-2-2011 23:19, for...@lyx.org schreef: Author: forenr Date: Wed Feb 16 23:19:49 2011 New Revision: 37704 URL: http://www.lyx.org/trac/changeset/37704 Log: Don't disable apply button if one (or more) of vertical alignment, rotation, or long table settings are changed. Fixes bug #7308. Modified: lyx-devel/trunk/src/insets/InsetTabular.cpp Modified: lyx-devel/trunk/src/insets/InsetTabular.cpp == --- lyx-devel/trunk/src/insets/InsetTabular.cpp Wed Feb 16 22:08:18 2011 (r37703) +++ lyx-devel/trunk/src/insets/InsetTabular.cpp Wed Feb 16 23:19:49 2011 (r37704) @@ -4323,8 +4323,10 @@ return true; case Tabular::SET_TABULAR_WIDTH: - status.setEnabled(!tabular.rotate&& !tabular.is_long_tabular - && tabular.tabular_valignment == Tabular::LYX_VALIGN_MIDDLE); + status.setEnabled(tabular.tabular_width.zero() + || (!tabular.rotate&& !tabular.is_long_tabular + && tabular.tabular_valignment == + Tabular::LYX_VALIGN_MIDDLE)); break; case Tabular::SET_DECIMAL_POINT: This is not correct. This is basically saying that SET_TABULAR_WIDTH should be allowed for longtables, because tabular_width is zero by definition for longtables. Vincent
