Re: [patch] do not allow too much demote in TOC dialog
There is no bugzilla entry. I found it from mailing list.. http://marc.info/?l=lyx-develm=117880601303849w=2 On 5/15/07, Jürgen Spitzmüller [EMAIL PROTECTED] wrote: José Matos wrote: I agree. +1 Committed. Was there a bugzilla entry about this? I thought so, but I don't find it now. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
There is no bugzilla entry. I found it from mailing list.. http://marc.info/?l=lyx-devel=117880601303849=2 On 5/15/07, Jürgen Spitzmüller <[EMAIL PROTECTED]> wrote: José Matos wrote: > > I agree. > > +1 Committed. Was there a bugzilla entry about this? I thought so, but I don't find it now. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Edwin Leuven wrote: i was thinking that if you create an LFUN_OUTLINE_IN action that you then associate with the demote button it would automatically be disabled when demoting is not possible. this way it would not be possible to click the button too many times... I think Ugras is correct. In the minibuffer, you can still promote/demote headings that are not numbered anymore, it's just the dialog that doesn't allow for this. So the lfun shouldn't be restricted. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote:
strange.. nobody complained the code below..
+
moveInTB-setEnabled(form_-allowDemoteCurrentItem(typeCO-currentIndex())
+ moveOutTB-isEnabled()); //means controls are
enabled.
I do ;-) (comments above the code please). And please stick to LyX's
conventions wrt line length and spacing, i.e.
return (kernel().buffer().params().tocdepth
- (*getCurrentTocItem(type)).depth() + 1) 0;
Most importantly, though, your patches crashes LyX for me on demoting over the
limit:
/usr/include/c++/4.1.2/debug/safe_iterator.h:181:error: attempt to
dereference a past-the-end iterator.
Objects involved in the operation:
iterator this @ 0x0x7fffc5bbabf0 {
type =
N11__gnu_debug14_Safe_iteratorIN9__gnu_cxx17__normal_iteratorIPKN3lyx7TocItemEN10__gnu_norm6vectorIS4_SaIS4_EN15__gnu_debug_def6vectorIS4_S9_
(constant iterator);
state = past-the-end;
references sequence with type
`N15__gnu_debug_def6vectorIN3lyx7TocItemESaIS2_EEE' @ 0x0x7fffc5bbabf0
}
Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: When demote button is pressed too many times, TOC item dissapears from the toc dialog, since it is no longer numbered. Hence, promoting back to the original state from TOC dialog is not possible. Attached patch is a way of correcting this behaviour. It simply prevents demoting the item more than document default max toc depth. What is your idea about the patch? I didn't look at the patch yet but I don't think this is the right thing to do. I'd prefer to properly allow non numbered items in the toc. So for me the correct fix is to not limit the TOC depth at all and to allow non numbered item in the TocBackend. So, if there is something to fix, it is not in the TocWidget but in the TocBackend. Abdel.
Re: [patch] do not allow too much demote in TOC dialog
Jürgen Spitzmüller wrote: Edwin Leuven wrote: i was thinking that if you create an LFUN_OUTLINE_IN action that you then associate with the demote button it would automatically be disabled when demoting is not possible. this way it would not be possible to click the button too many times... I think Ugras is correct. In the minibuffer, you can still promote/demote headings that are not numbered anymore, it's just the dialog that doesn't allow for this. So the lfun shouldn't be restricted. It's not the dialog but the TocBackend that doesn't allow this. Abdel.
Re: [patch] do not allow too much demote in TOC dialog
I can't reproduce this. I even tried inline-in command from buffer.
One point is, if you have older versions of toc stuff (pre 18265) and apply
this patch, it is possible to get such errors *I have tested and found
some). Are you sure that you have cleanly updated to the current svn?
I will re-send the patch with your comments applied, after coding the
proposed solution of Abdel.
Ugras
On 5/15/07, Jürgen Spitzmüller [EMAIL PROTECTED] wrote:
Ozgur Ugras BARAN wrote:
strange.. nobody complained the code below..
+
moveInTB-setEnabled(form_-allowDemoteCurrentItem(typeCO-currentIndex())
+ moveOutTB-isEnabled()); //means controls are
enabled.
I do ;-) (comments above the code please). And please stick to LyX's
conventions wrt line length and spacing, i.e.
return (kernel().buffer().params().tocdepth
- (*getCurrentTocItem(type)).depth() + 1) 0;
Most importantly, though, your patches crashes LyX for me on demoting over
the
limit:
/usr/include/c++/4.1.2/debug/safe_iterator.h:181:error: attempt to
dereference a past-the-end iterator.
Objects involved in the operation:
iterator this @ 0x0x7fffc5bbabf0 {
type =
N11__gnu_debug14_Safe_iteratorIN9__gnu_cxx17__normal_iteratorIPKN3lyx7TocItemEN10__gnu_norm6vectorIS4_SaIS4_EN15__gnu_debug_def6vectorIS4_S9_
(constant iterator);
state = past-the-end;
references sequence with type
`N15__gnu_debug_def6vectorIN3lyx7TocItemESaIS2_EEE' @ 0x0x7fffc5bbabf0
}
Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: It is strictly dialog related problem, therefore I should have put some function in controller. The place for a flag like demotionEnabled maybe in TocBackend, but this doesn't remove the necessity of a controller function. IMHO, the code is cleaner as it is now. My question was different, actually. What I am doing now is to prevent demotion from the dialog. Another option would be not to filter TOC entries in TOC dialog, therefore, demotion will never be a problem. The downside of this method is over-crowded TOC dialog. Will that crowding be a problem? Only seeing the numbered entries looks like a somewhat arbitrary limitation. Of course, users then have configurability - they can see everything by temporarily making everything numbered. Another idea: Put in a slider so the user can decide how many levels to see. Default to all, or perhaps to the numbered levels. The user can then change to whatever he likes. Helge Hafting
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: I can't reproduce this. I even tried inline-in command from buffer. One point is, if you have older versions of toc stuff (pre 18265) and apply this patch, it is possible to get such errors *I have tested and found some). Are you sure that you have cleanly updated to the current svn? Yes. However, I have stdlib-debug enabled. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Umm. let me check, then. How can I enable stdlib-debug ? Ugras On 5/15/07, Jürgen Spitzmüller [EMAIL PROTECTED] wrote: Ozgur Ugras BARAN wrote: I can't reproduce this. I even tried inline-in command from buffer. One point is, if you have older versions of toc stuff (pre 18265) and apply this patch, it is possible to get such errors *I have tested and found some). Are you sure that you have cleanly updated to the current svn? Yes. However, I have stdlib-debug enabled. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: How can I enable stdlib-debug ? --enable-stdlib-debug Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Well, I don't know. Please try yourself and let me know. Attached is
two patches: First patch (toc.diff) prevents over-demoting and second
one (TocBackend.diff), puts everything in TOC. (Abdel, This was what
you had asked, isn't it?)
In my opinion, numbered TOC entries are structural elemens of a
document. Non-numbered ones are used mostly for categorization
elements. Therefore I vote for the first patch.
I don't think putting another slider in dialog area is a good idea in
terms of simpllicity of dialog. Similar settings are made in
Document/Settings/Numbering TOC, which is correct place for this
IMHO.
regards,
Ugras
On 5/15/07, Helge Hafting [EMAIL PROTECTED] wrote:
Ozgur Ugras BARAN wrote:
It is strictly dialog related problem, therefore I should have put
some function in controller. The place for a flag like demotionEnabled
maybe in TocBackend, but this doesn't remove the necessity of a
controller function. IMHO, the code is cleaner as it is now.
My question was different, actually. What I am doing now is to prevent
demotion from the dialog. Another option would be not to filter TOC
entries in TOC dialog, therefore, demotion will never be a problem.
The downside of this method is over-crowded TOC dialog.
Will that crowding be a problem?
Only seeing the numbered entries looks like a somewhat arbitrary
limitation. Of course, users then have configurability - they can see
everything by temporarily making everything numbered.
Another idea: Put in a slider so the user can decide how many levels
to see. Default to all, or perhaps to the numbered levels. The
user can then change to whatever he likes.
Helge Hafting
Index: frontends/qt4/TocWidget.cpp
===
--- frontends/qt4/TocWidget.cpp (revision 18311)
+++ frontends/qt4/TocWidget.cpp (working copy)
@@ -211,6 +211,8 @@
tocTV-selectionModel()-setCurrentIndex(index,
QItemSelectionModel::ClearAndSelect);
tocTV-selectionModel()-blockSignals(false);
+ moveInTB-setEnabled(moveOutTB-isEnabled()
+ form_-allowDemoteCurrentItem(typeCO-currentIndex()));
}
@@ -223,7 +225,8 @@
moveUpTB-setEnabled(enable);
moveDownTB-setEnabled(enable);
- moveInTB-setEnabled(enable);
+ moveInTB-setEnabled(enable
+ form_-allowDemoteCurrentItem(typeCO-currentIndex()));
moveOutTB-setEnabled(enable);
depthSL-setEnabled(enable);
Index: frontends/controllers/ControlToc.cpp
===
--- frontends/controllers/ControlToc.cpp (revision 18311)
+++ frontends/controllers/ControlToc.cpp (working copy)
@@ -142,5 +142,11 @@
return _(type);
}
+bool ControlToc::allowDemoteCurrentItem(size_t type) const
+{
+ return (kernel().buffer().params().tocdepth
+ - getCurrentTocItem(type)-depth() + 1) 0;
+}
+
} // namespace frontend
} // namespace lyx
Index: frontends/controllers/ControlToc.h
===
--- frontends/controllers/ControlToc.h (revision 18311)
+++ frontends/controllers/ControlToc.h (working copy)
@@ -62,6 +62,8 @@
bool canOutline(size_t type) const;
///
void updateBackend();
+ /// Is current item depth is equal the max allowed toc depth
+ bool allowDemoteCurrentItem(size_t type_) const;
private:
/// Return the guiname from a given cmdName of the TOC param
Index: TocBackend.cpp
===
--- TocBackend.cpp (revision 18311)
+++ TocBackend.cpp (working copy)
@@ -156,7 +156,6 @@
int const toclevel = toc_item-par_it_-layout()-toclevel;
if (toclevel != Layout::NOT_IN_TOC
toclevel = min_toclevel
- toclevel = bufparams.tocdepth
tocstring.empty())
tocstring = toc_item-par_it_-asString(*buffer_, true);
@@ -205,8 +204,7 @@
/// now the toc entry for the paragraph
int const toclevel = pit-layout()-toclevel;
if (toclevel != Layout::NOT_IN_TOC
- toclevel = min_toclevel
- toclevel = bufparams.tocdepth) {
+ toclevel = min_toclevel) {
// insert this into the table of contents
if (tocstring.empty())
tocstring = pit-asString(*buffer_, true);
Re: [patch] do not allow too much demote in TOC dialog
Ozgur == Ozgur Ugras BARAN [EMAIL PROTECTED] writes: Ozgur Well, I don't know. Please try yourself and let me know. Ozgur Attached is two patches: First patch (toc.diff) prevents Ozgur over-demoting and second one (TocBackend.diff), puts everything Ozgur in TOC. (Abdel, This was what you had asked, isn't it?) Ozgur In my opinion, numbered TOC entries are structural elemens of a Ozgur document. Non-numbered ones are used mostly for categorization Ozgur elements. Therefore I vote for the first patch. I disagree. All sectional headings are part of the structure and should be available through the outline panel. I do not think you should restrict usage according to a single view. Ozgur I don't think putting another slider in dialog area is a good Ozgur idea in terms of simpllicity of dialog. Similar settings are Ozgur made in Document/Settings/Numbering TOC, which is correct Ozgur place for this IMHO. No, because this modifies the document. A separate slider would allow to see whatever structure the author wants to see.
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: Well, I don't know. Please try yourself and let me know. Attached is two patches: First patch (toc.diff) prevents over-demoting and second one (TocBackend.diff), puts everything in TOC. (Abdel, This was what you had asked, isn't it?) Yes. In my opinion, numbered TOC entries are structural elemens of a document. Non-numbered ones are used mostly for categorization elements. Therefore I vote for the first patch. I vote for the second ;-) Non-numbered items are certainly giving structure to a document. Think novels. Besides, the second patch is way simpler and logical. I don't think putting another slider in dialog area is a good idea in terms of simpllicity of dialog. Similar settings are made in Document/Settings/Numbering TOC, which is correct place for this IMHO. The current slider is fine for that purpose, why another one? Abdel.
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: Well, I don't know. Please try yourself and let me know. Attached is two patches: First patch (toc.diff) prevents over-demoting and second one (TocBackend.diff), puts everything in TOC. (Abdel, This was what you had asked, isn't it?) No need for a patch to test everything in TOC, I can test that effect by setting everything to numbered in the document settings. In my opinion, numbered TOC entries are structural elemens of a document. Non-numbered ones are used mostly for categorization elements. Therefore I vote for the first patch. This is correct for documents that actually use numbers. Some kinds of literature don't, or perhaps only chapters are numbered. It is still nice to be able to rearrange things. I don't think putting another slider in dialog area is a good idea in terms of simpllicity of dialog. Similar settings are made in Document/Settings/Numbering TOC, which is correct place for this IMHO. I disagree. The settings in Numbering decides what headings get printed with a number, and what headings get in the printed TOC. A third slider there, that don't affect the document (just the TOC dialog) would be * confusing - people not using the TOC dialog will see it * cumbersome - people using the TOC will need to bring up another dialog A slider with the caption Levels to show here (or perhaps some better english) surely is one more item in the dialog. But this is where it belongs, because it affects nothing but the dialog itself. It is easy for the user to see what it does by experimenting. It can also be safely ignored for beginners. It won't get in the way when the TOC dialog isn't in use. Having the numbered levels as a default setting will give todays setup by default. But the users can then customize. Some will find it too crowded when the dialog shows numbered subsubsections, other will enable more for finer control. Helge Hafting
Re: [patch] do not allow too much demote in TOC dialog
Gosh.. this takes hours to link.. I have tested, but no crashes.. sorry. I will test it again tonight. regards, ugras On 5/15/07, Jürgen Spitzmüller [EMAIL PROTECTED] wrote: Ozgur Ugras BARAN wrote: How can I enable stdlib-debug ? --enable-stdlib-debug Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Abdelrazak Younes wrote: In my opinion, numbered TOC entries are structural elemens of a document. Non-numbered ones are used mostly for categorization elements. Therefore I vote for the first patch. I vote for the second ;-) +1 I don't think putting another slider in dialog area is a good idea in terms of simpllicity of dialog. Similar settings are made in Document/Settings/Numbering TOC, which is correct place for this IMHO. The current slider is fine for that purpose, why another one? +1 Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: Gosh.. this takes hours to link.. I have tested, but no crashes.. sorry. I will test it again tonight. Maybe my system is pickier (64bit). Anyway, seems your other (TOC backend) patch is getting more sympathy anyway. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
On 5/15/07, Jürgen Spitzmüller [EMAIL PROTECTED] wrote: Ozgur Ugras BARAN wrote: Gosh.. this takes hours to link.. I have tested, but no crashes.. sorry. I will test it again tonight. Maybe my system is pickier (64bit). Anyway, seems your other (TOC backend) patch is getting more sympathy anyway. I figured out :).. But I am a bit obsessive I guess. :) Jürgen So.. It seems there is a consensus on the second approach. (which is also fine for me). Then, how about puttimg it in svn? If it will be the case, here is svn log: * src/TocBackend.cpp: -Allow TOC entries with depth larger than bufferParams.tocdepth.
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: So.. It seems there is a consensus on the second approach. (which is also fine for me). Then, how about puttimg it in svn? Jean-Marc? José? Jürgen
Re: [patch] do not allow too much demote in TOC dialog
On Tuesday 15 May 2007 15:53:53 Jean-Marc Lasgouttes wrote: I agree. +1 JMarc -- José Abílio
Re: [patch] do not allow too much demote in TOC dialog
Jürgen == Jürgen Spitzmüller [EMAIL PROTECTED] writes: Jürgen Ozgur Ugras BARAN wrote: So.. It seems there is a consensus on the second approach. (which is also fine for me). Then, how about puttimg it in svn? Jürgen Jean-Marc? José? I agree. JMarc
Re: [patch] do not allow too much demote in TOC dialog
José Matos wrote: I agree. +1 Committed. Was there a bugzilla entry about this? I thought so, but I don't find it now. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Edwin Leuven wrote: > i was thinking that if you create an LFUN_OUTLINE_IN action that you > then associate with the demote button it would automatically be disabled > when demoting is not possible. this way it would not be possible to > click the button "too many times"... I think Ugras is correct. In the minibuffer, you can still promote/demote headings that are not numbered anymore, it's just the dialog that doesn't allow for this. So the lfun shouldn't be restricted. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote:
> strange.. nobody complained the code below..
>
> +
> moveInTB->setEnabled(form_->allowDemoteCurrentItem(typeCO->currentIndex())
> &&
> + moveOutTB->isEnabled()); //means controls are
> enabled.
I do ;-) (comments above the code please). And please stick to LyX's
conventions wrt line length and spacing, i.e.
return (kernel().buffer().params().tocdepth
- (*getCurrentTocItem(type)).depth() + 1) > 0;
Most importantly, though, your patches crashes LyX for me on demoting over the
limit:
/usr/include/c++/4.1.2/debug/safe_iterator.h:181:error: attempt to
dereference a past-the-end iterator.
Objects involved in the operation:
iterator "this" @ 0x0x7fffc5bbabf0 {
type =
N11__gnu_debug14_Safe_iteratorIN9__gnu_cxx17__normal_iteratorIPKN3lyx7TocItemEN10__gnu_norm6vectorIS4_SaIS4_EN15__gnu_debug_def6vectorIS4_S9_
(constant iterator);
state = past-the-end;
references sequence with type
`N15__gnu_debug_def6vectorIN3lyx7TocItemESaIS2_EEE' @ 0x0x7fffc5bbabf0
}
Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: When demote button is pressed too many times, TOC item dissapears from the toc dialog, since it is no longer numbered. Hence, promoting back to the original state from TOC dialog is not possible. Attached patch is a way of correcting this behaviour. It simply prevents demoting the item more than document default max toc depth. What is your idea about the patch? I didn't look at the patch yet but I don't think this is the right thing to do. I'd prefer to properly allow non numbered items in the toc. So for me the correct fix is to not limit the TOC depth at all and to allow non numbered item in the TocBackend. So, if there is something to fix, it is not in the TocWidget but in the TocBackend. Abdel.
Re: [patch] do not allow too much demote in TOC dialog
Jürgen Spitzmüller wrote: Edwin Leuven wrote: i was thinking that if you create an LFUN_OUTLINE_IN action that you then associate with the demote button it would automatically be disabled when demoting is not possible. this way it would not be possible to click the button "too many times"... I think Ugras is correct. In the minibuffer, you can still promote/demote headings that are not numbered anymore, it's just the dialog that doesn't allow for this. So the lfun shouldn't be restricted. It's not the dialog but the TocBackend that doesn't allow this. Abdel.
Re: [patch] do not allow too much demote in TOC dialog
I can't reproduce this. I even tried inline-in command from buffer.
One point is, if you have older versions of toc stuff (pre 18265) and apply
this patch, it is possible to get such errors *I have tested and found
some). Are you sure that you have cleanly updated to the current svn?
I will re-send the patch with your comments applied, after coding the
proposed solution of Abdel.
Ugras
On 5/15/07, Jürgen Spitzmüller <[EMAIL PROTECTED]> wrote:
Ozgur Ugras BARAN wrote:
> strange.. nobody complained the code below..
>
> +
>
moveInTB->setEnabled(form_->allowDemoteCurrentItem(typeCO->currentIndex())
> &&
> + moveOutTB->isEnabled()); //means controls are
> enabled.
I do ;-) (comments above the code please). And please stick to LyX's
conventions wrt line length and spacing, i.e.
return (kernel().buffer().params().tocdepth
- (*getCurrentTocItem(type)).depth() + 1) > 0;
Most importantly, though, your patches crashes LyX for me on demoting over
the
limit:
/usr/include/c++/4.1.2/debug/safe_iterator.h:181:error: attempt to
dereference a past-the-end iterator.
Objects involved in the operation:
iterator "this" @ 0x0x7fffc5bbabf0 {
type =
N11__gnu_debug14_Safe_iteratorIN9__gnu_cxx17__normal_iteratorIPKN3lyx7TocItemEN10__gnu_norm6vectorIS4_SaIS4_EN15__gnu_debug_def6vectorIS4_S9_
(constant iterator);
state = past-the-end;
references sequence with type
`N15__gnu_debug_def6vectorIN3lyx7TocItemESaIS2_EEE' @ 0x0x7fffc5bbabf0
}
Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: It is strictly dialog related problem, therefore I should have put some function in controller. The place for a flag like demotionEnabled maybe in TocBackend, but this doesn't remove the necessity of a controller function. IMHO, the code is cleaner as it is now. My question was different, actually. What I am doing now is to prevent demotion from the dialog. Another option would be not to filter TOC entries in TOC dialog, therefore, demotion will never be a problem. The downside of this method is over-crowded TOC dialog. Will that "crowding" be a problem? Only seeing the numbered entries looks like a somewhat arbitrary limitation. Of course, users then have configurability - they can see everything by temporarily making everything numbered. Another idea: Put in a slider so the user can decide how many levels to see. Default to all, or perhaps to the numbered levels. The user can then change to whatever he likes. Helge Hafting
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: > I can't reproduce this. I even tried inline-in command from buffer. > > One point is, if you have older versions of toc stuff (pre 18265) and apply > this patch, it is possible to get such errors *I have tested and found > some). Are you sure that you have cleanly updated to the current svn? Yes. However, I have stdlib-debug enabled. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Umm. let me check, then. How can I enable stdlib-debug ? Ugras On 5/15/07, Jürgen Spitzmüller <[EMAIL PROTECTED]> wrote: Ozgur Ugras BARAN wrote: > I can't reproduce this. I even tried inline-in command from buffer. > > One point is, if you have older versions of toc stuff (pre 18265) and apply > this patch, it is possible to get such errors *I have tested and found > some). Are you sure that you have cleanly updated to the current svn? Yes. However, I have stdlib-debug enabled. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: > How can I enable stdlib-debug ? --enable-stdlib-debug Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Well, I don't know. Please try yourself and let me know. Attached is
two patches: First patch (toc.diff) prevents over-demoting and second
one (TocBackend.diff), puts everything in TOC. (Abdel, This was what
you had asked, isn't it?)
In my opinion, numbered TOC entries are structural elemens of a
document. Non-numbered ones are used mostly for categorization
elements. Therefore I vote for the first patch.
I don't think putting another slider in dialog area is a good idea in
terms of simpllicity of dialog. Similar settings are made in
Document/Settings/Numbering & TOC, which is correct place for this
IMHO.
regards,
Ugras
On 5/15/07, Helge Hafting <[EMAIL PROTECTED]> wrote:
Ozgur Ugras BARAN wrote:
> It is strictly dialog related problem, therefore I should have put
> some function in controller. The place for a flag like demotionEnabled
> maybe in TocBackend, but this doesn't remove the necessity of a
> controller function. IMHO, the code is cleaner as it is now.
>
> My question was different, actually. What I am doing now is to prevent
> demotion from the dialog. Another option would be not to filter TOC
> entries in TOC dialog, therefore, demotion will never be a problem.
> The downside of this method is over-crowded TOC dialog.
Will that "crowding" be a problem?
Only seeing the numbered entries looks like a somewhat arbitrary
limitation. Of course, users then have configurability - they can see
everything by temporarily making everything numbered.
Another idea: Put in a slider so the user can decide how many levels
to see. Default to all, or perhaps to the numbered levels. The
user can then change to whatever he likes.
Helge Hafting
Index: frontends/qt4/TocWidget.cpp
===
--- frontends/qt4/TocWidget.cpp (revision 18311)
+++ frontends/qt4/TocWidget.cpp (working copy)
@@ -211,6 +211,8 @@
tocTV->selectionModel()->setCurrentIndex(index,
QItemSelectionModel::ClearAndSelect);
tocTV->selectionModel()->blockSignals(false);
+ moveInTB->setEnabled(moveOutTB->isEnabled() &&
+ form_->allowDemoteCurrentItem(typeCO->currentIndex()));
}
@@ -223,7 +225,8 @@
moveUpTB->setEnabled(enable);
moveDownTB->setEnabled(enable);
- moveInTB->setEnabled(enable);
+ moveInTB->setEnabled(enable &&
+ form_->allowDemoteCurrentItem(typeCO->currentIndex()));
moveOutTB->setEnabled(enable);
depthSL->setEnabled(enable);
Index: frontends/controllers/ControlToc.cpp
===
--- frontends/controllers/ControlToc.cpp (revision 18311)
+++ frontends/controllers/ControlToc.cpp (working copy)
@@ -142,5 +142,11 @@
return _(type);
}
+bool ControlToc::allowDemoteCurrentItem(size_t type) const
+{
+ return (kernel().buffer().params().tocdepth
+ - getCurrentTocItem(type)->depth() + 1) > 0;
+}
+
} // namespace frontend
} // namespace lyx
Index: frontends/controllers/ControlToc.h
===
--- frontends/controllers/ControlToc.h (revision 18311)
+++ frontends/controllers/ControlToc.h (working copy)
@@ -62,6 +62,8 @@
bool canOutline(size_t type) const;
///
void updateBackend();
+ /// Is current item depth is equal the max allowed toc depth
+ bool allowDemoteCurrentItem(size_t type_) const;
private:
/// Return the guiname from a given cmdName of the TOC param
Index: TocBackend.cpp
===
--- TocBackend.cpp (revision 18311)
+++ TocBackend.cpp (working copy)
@@ -156,7 +156,6 @@
int const toclevel = toc_item->par_it_->layout()->toclevel;
if (toclevel != Layout::NOT_IN_TOC
&& toclevel >= min_toclevel
- && toclevel <= bufparams.tocdepth
&& tocstring.empty())
tocstring = toc_item->par_it_->asString(*buffer_, true);
@@ -205,8 +204,7 @@
/// now the toc entry for the paragraph
int const toclevel = pit->layout()->toclevel;
if (toclevel != Layout::NOT_IN_TOC
- && toclevel >= min_toclevel
- && toclevel <= bufparams.tocdepth) {
+ && toclevel >= min_toclevel) {
// insert this into the table of contents
if (tocstring.empty())
tocstring = pit->asString(*buffer_, true);
Re: [patch] do not allow too much demote in TOC dialog
> "Ozgur" == Ozgur Ugras BARAN <[EMAIL PROTECTED]> writes: Ozgur> Well, I don't know. Please try yourself and let me know. Ozgur> Attached is two patches: First patch (toc.diff) prevents Ozgur> over-demoting and second one (TocBackend.diff), puts everything Ozgur> in TOC. (Abdel, This was what you had asked, isn't it?) Ozgur> In my opinion, numbered TOC entries are structural elemens of a Ozgur> document. Non-numbered ones are used mostly for categorization Ozgur> elements. Therefore I vote for the first patch. I disagree. All sectional headings are part of the structure and should be available through the outline panel. I do not think you should restrict usage according to a single view. Ozgur> I don't think putting another slider in dialog area is a good Ozgur> idea in terms of simpllicity of dialog. Similar settings are Ozgur> made in Document/Settings/Numbering & TOC, which is correct Ozgur> place for this IMHO. No, because this modifies the document. A separate slider would allow to see whatever structure the author wants to see.
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: Well, I don't know. Please try yourself and let me know. Attached is two patches: First patch (toc.diff) prevents over-demoting and second one (TocBackend.diff), puts everything in TOC. (Abdel, This was what you had asked, isn't it?) Yes. In my opinion, numbered TOC entries are structural elemens of a document. Non-numbered ones are used mostly for categorization elements. Therefore I vote for the first patch. I vote for the second ;-) Non-numbered items are certainly giving structure to a document. Think novels. Besides, the second patch is way simpler and logical. I don't think putting another slider in dialog area is a good idea in terms of simpllicity of dialog. Similar settings are made in Document/Settings/Numbering & TOC, which is correct place for this IMHO. The current slider is fine for that purpose, why another one? Abdel.
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: Well, I don't know. Please try yourself and let me know. Attached is two patches: First patch (toc.diff) prevents over-demoting and second one (TocBackend.diff), puts everything in TOC. (Abdel, This was what you had asked, isn't it?) No need for a patch to test "everything in TOC", I can test that effect by setting everything to numbered in the document settings. In my opinion, numbered TOC entries are structural elemens of a document. Non-numbered ones are used mostly for categorization elements. Therefore I vote for the first patch. This is correct for documents that actually use numbers. Some kinds of literature don't, or perhaps only chapters are numbered. It is still nice to be able to rearrange things. I don't think putting another slider in dialog area is a good idea in terms of simpllicity of dialog. Similar settings are made in Document/Settings/Numbering & TOC, which is correct place for this IMHO. I disagree. The settings in "Numbering" decides what headings get printed with a number, and what headings get in the printed TOC. A third slider there, that don't affect the document (just the TOC dialog) would be * confusing - people not using the TOC dialog will see it * cumbersome - people using the TOC will need to bring up another dialog A slider with the caption "Levels to show here" (or perhaps some better english) surely is one more item in the dialog. But this is where it belongs, because it affects nothing but the dialog itself. It is easy for the user to see what it does by experimenting. It can also be safely ignored for beginners. It won't get in the way when the TOC dialog isn't in use. Having the numbered levels as a default setting will give todays setup by default. But the users can then customize. Some will find it too crowded when the dialog shows numbered subsubsections, other will enable more for finer control. Helge Hafting
Re: [patch] do not allow too much demote in TOC dialog
Gosh.. this takes hours to link.. I have tested, but no crashes.. sorry. I will test it again tonight. regards, ugras On 5/15/07, Jürgen Spitzmüller <[EMAIL PROTECTED]> wrote: Ozgur Ugras BARAN wrote: > How can I enable stdlib-debug ? --enable-stdlib-debug Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Abdelrazak Younes wrote: > > In my opinion, numbered TOC entries are structural elemens of a > > document. Non-numbered ones are used mostly for categorization > > elements. Therefore I vote for the first patch. > > I vote for the second ;-) +1 > > I don't think putting another slider in dialog area is a good idea in > > terms of simpllicity of dialog. Similar settings are made in > > Document/Settings/Numbering & TOC, which is correct place for this > > IMHO. > > The current slider is fine for that purpose, why another one? +1 Jürgen
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: > Gosh.. this takes hours to link.. > > I have tested, but no crashes.. sorry. I will test it again tonight. Maybe my system is pickier (64bit). Anyway, seems your other (TOC backend) patch is getting more sympathy anyway. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
On 5/15/07, Jürgen Spitzmüller <[EMAIL PROTECTED]> wrote: Ozgur Ugras BARAN wrote: > Gosh.. this takes hours to link.. > > I have tested, but no crashes.. sorry. I will test it again tonight. Maybe my system is pickier (64bit). Anyway, seems your other (TOC backend) patch is getting more sympathy anyway. I figured out :).. But I am a bit obsessive I guess. :) Jürgen So.. It seems there is a consensus on the second approach. (which is also fine for me). Then, how about puttimg it in svn? If it will be the case, here is svn log: * src/TocBackend.cpp: -Allow TOC entries with depth larger than bufferParams.tocdepth.
Re: [patch] do not allow too much demote in TOC dialog
Ozgur Ugras BARAN wrote: > So.. It seems there is a consensus on the second approach. (which is > also fine for me). Then, how about puttimg it in svn? Jean-Marc? José? Jürgen
Re: [patch] do not allow too much demote in TOC dialog
On Tuesday 15 May 2007 15:53:53 Jean-Marc Lasgouttes wrote: > > I agree. +1 > JMarc -- José Abílio
Re: [patch] do not allow too much demote in TOC dialog
> "Jürgen" == Jürgen Spitzmüller <[EMAIL PROTECTED]> writes: Jürgen> Ozgur Ugras BARAN wrote: >> So.. It seems there is a consensus on the second approach. (which >> is also fine for me). Then, how about puttimg it in svn? Jürgen> Jean-Marc? José? I agree. JMarc
Re: [patch] do not allow too much demote in TOC dialog
José Matos wrote: > > I agree. > > +1 Committed. Was there a bugzilla entry about this? I thought so, but I don't find it now. Jürgen
Re: [patch] do not allow too much demote in TOC dialog
shouldn't the proper enabled flag be set in the kernel (bufferview.cpp?) instead of putting this in the controller? Ozgur Ugras BARAN wrote: When demote button is pressed too many times, TOC item dissapears from the toc dialog, since it is no longer numbered. Hence, promoting back to the original state from TOC dialog is not possible. Attached patch is a way of correcting this behaviour. It simply prevents demoting the item more than document default max toc depth. What is your idea about the patch? regards Ugras
Re: [patch] do not allow too much demote in TOC dialog
On Mon, May 14, 2007 at 08:19:59PM +0200, Ozgur Ugras BARAN wrote:
+bool const ControlToc::allowDemoteCurrentItem(size_t type) const
+{
+ return ((kernel().buffer().params().tocdepth -
(*getCurrentTocItem(type)).depth() + 1)0);
+}
On the formal side:
No need for the outer parantheses.
I moreover personally find a + 1 b easier to read then ((a - (b) + 1)0)
There is no need for the 'const' on the return value either.
Ander'
Re: [patch] do not allow too much demote in TOC dialog
It is strictly dialog related problem, therefore I should have put some function in controller. The place for a flag like demotionEnabled maybe in TocBackend, but this doesn't remove the necessity of a controller function. IMHO, the code is cleaner as it is now. My question was different, actually. What I am doing now is to prevent demotion from the dialog. Another option would be not to filter TOC entries in TOC dialog, therefore, demotion will never be a problem. The downside of this method is over-crowded TOC dialog. I wonder what people think about current solution? Does anybody have another idea? Ugras On 5/14/07, Edwin Leuven [EMAIL PROTECTED] wrote: shouldn't the proper enabled flag be set in the kernel (bufferview.cpp?) instead of putting this in the controller? Ozgur Ugras BARAN wrote: When demote button is pressed too many times, TOC item dissapears from the toc dialog, since it is no longer numbered. Hence, promoting back to the original state from TOC dialog is not possible. Attached patch is a way of correcting this behaviour. It simply prevents demoting the item more than document default max toc depth. What is your idea about the patch? regards Ugras
Re: [patch] do not allow too much demote in TOC dialog
i probably don't understand you. i was thinking that if you create an LFUN_OUTLINE_IN action that you then associate with the demote button it would automatically be disabled when demoting is not possible. this way it would not be possible to click the button too many times... anyway, just a thought that crossed my mind (don't know the toc code...) Ozgur Ugras BARAN wrote: It is strictly dialog related problem, therefore I should have put some function in controller. The place for a flag like demotionEnabled maybe in TocBackend, but this doesn't remove the necessity of a controller function. IMHO, the code is cleaner as it is now. My question was different, actually. What I am doing now is to prevent demotion from the dialog. Another option would be not to filter TOC entries in TOC dialog, therefore, demotion will never be a problem. The downside of this method is over-crowded TOC dialog. I wonder what people think about current solution? Does anybody have another idea? Ugras On 5/14/07, Edwin Leuven [EMAIL PROTECTED] wrote: shouldn't the proper enabled flag be set in the kernel (bufferview.cpp?) instead of putting this in the controller? Ozgur Ugras BARAN wrote: When demote button is pressed too many times, TOC item dissapears from the toc dialog, since it is no longer numbered. Hence, promoting back to the original state from TOC dialog is not possible. Attached patch is a way of correcting this behaviour. It simply prevents demoting the item more than document default max toc depth. What is your idea about the patch? regards Ugras
Re: [patch] do not allow too much demote in TOC dialog
On 5/14/07, Andre Poenitz [EMAIL PROTECTED] wrote:
On Mon, May 14, 2007 at 08:19:59PM +0200, Ozgur Ugras BARAN wrote:
+bool const ControlToc::allowDemoteCurrentItem(size_t type) const
+{
+ return ((kernel().buffer().params().tocdepth -
(*getCurrentTocItem(type)).depth() + 1)0);
+}
On the formal side:
No need for the outer parantheses.
removed
I moreover personally find a + 1 b easier to read then ((a - (b) +
1)0)
I guess I prefer (a - b +1) 0. (paranthesis around
(*getCurrentTocItem(type)) is necessary here)
There is no need for the 'const' on the return value either.
removed
Ander'
strange.. nobody complained the code below..
+
moveInTB-setEnabled(form_-allowDemoteCurrentItem(typeCO-currentIndex())
+ moveOutTB-isEnabled()); //means controls are enabled.
regards
Ugras
Index: frontends/qt4/TocWidget.cpp
===
--- frontends/qt4/TocWidget.cpp (revision 18311)
+++ frontends/qt4/TocWidget.cpp (working copy)
@@ -211,6 +211,8 @@
tocTV-selectionModel()-setCurrentIndex(index,
QItemSelectionModel::ClearAndSelect);
tocTV-selectionModel()-blockSignals(false);
+ moveInTB-setEnabled(form_-allowDemoteCurrentItem(typeCO-currentIndex())
+ moveOutTB-isEnabled()); //means controls are enabled.
}
@@ -223,7 +225,8 @@
moveUpTB-setEnabled(enable);
moveDownTB-setEnabled(enable);
- moveInTB-setEnabled(enable);
+ moveInTB-setEnabled(enable
+ form_-allowDemoteCurrentItem(typeCO-currentIndex()));
moveOutTB-setEnabled(enable);
depthSL-setEnabled(enable);
Index: frontends/controllers/ControlToc.cpp
===
--- frontends/controllers/ControlToc.cpp (revision 18311)
+++ frontends/controllers/ControlToc.cpp (working copy)
@@ -142,5 +142,10 @@
return _(type);
}
+bool ControlToc::allowDemoteCurrentItem(size_t type) const
+{
+ return (kernel().buffer().params().tocdepth - (*getCurrentTocItem(type)).depth() + 1)0;
+}
+
} // namespace frontend
} // namespace lyx
Index: frontends/controllers/ControlToc.h
===
--- frontends/controllers/ControlToc.h (revision 18311)
+++ frontends/controllers/ControlToc.h (working copy)
@@ -62,6 +62,8 @@
bool canOutline(size_t type) const;
///
void updateBackend();
+ /// Is current item depth is equal the max allowed toc depth
+ bool allowDemoteCurrentItem(size_t type_) const;
private:
/// Return the guiname from a given cmdName of the TOC param
Re: [patch] do not allow too much demote in TOC dialog
On Mon, May 14, 2007 at 09:47:11PM +0200, Ozgur Ugras BARAN wrote:
On 5/14/07, Andre Poenitz [EMAIL PROTECTED] wrote:
On Mon, May 14, 2007 at 08:19:59PM +0200, Ozgur Ugras BARAN wrote:
+bool const ControlToc::allowDemoteCurrentItem(size_t type) const
+{
+ return ((kernel().buffer().params().tocdepth -
(*getCurrentTocItem(type)).depth() + 1)0);
+}
On the formal side:
No need for the outer parantheses.
removed
I moreover personally find a + 1 b easier to read then ((a - (b) +
1)0)
I guess I prefer (a - b +1) 0. (paranthesis around
(*getCurrentTocItem(type)) is necessary here)
But only because you do not write
getCurrentTocItem(type)-depth()
Andre'
Re: [patch] do not allow too much demote in TOC dialog
shouldn't the proper "enabled" flag be set in the kernel (bufferview.cpp?) instead of putting this in the controller? Ozgur Ugras BARAN wrote: When demote button is pressed too many times, TOC item dissapears from the toc dialog, since it is no longer numbered. Hence, promoting back to the original state from TOC dialog is not possible. Attached patch is a way of correcting this behaviour. It simply prevents demoting the item more than document default max toc depth. What is your idea about the patch? regards Ugras
Re: [patch] do not allow too much demote in TOC dialog
On Mon, May 14, 2007 at 08:19:59PM +0200, Ozgur Ugras BARAN wrote:
> +bool const ControlToc::allowDemoteCurrentItem(size_t type) const
> +{
> + return ((kernel().buffer().params().tocdepth -
> (*getCurrentTocItem(type)).depth() + 1)>0);
> +}
On the formal side:
No need for the outer parantheses.
I moreover personally find a + 1 > b easier to read then ((a - (b) + 1)>0)
There is no need for the 'const' on the return value either.
Ander'
Re: [patch] do not allow too much demote in TOC dialog
It is strictly dialog related problem, therefore I should have put some function in controller. The place for a flag like demotionEnabled maybe in TocBackend, but this doesn't remove the necessity of a controller function. IMHO, the code is cleaner as it is now. My question was different, actually. What I am doing now is to prevent demotion from the dialog. Another option would be not to filter TOC entries in TOC dialog, therefore, demotion will never be a problem. The downside of this method is over-crowded TOC dialog. I wonder what people think about current solution? Does anybody have another idea? Ugras On 5/14/07, Edwin Leuven <[EMAIL PROTECTED]> wrote: shouldn't the proper "enabled" flag be set in the kernel (bufferview.cpp?) instead of putting this in the controller? Ozgur Ugras BARAN wrote: > When demote button is pressed too many times, TOC item dissapears from > the toc dialog, since it is no longer numbered. Hence, promoting back > to the original state from TOC dialog is not possible. Attached patch > is a way of correcting this behaviour. It simply prevents demoting the > item more than document default max toc depth. > > What is your idea about the patch? > > regards > > Ugras >
Re: [patch] do not allow too much demote in TOC dialog
i probably don't understand you. i was thinking that if you create an LFUN_OUTLINE_IN action that you then associate with the demote button it would automatically be disabled when demoting is not possible. this way it would not be possible to click the button "too many times"... anyway, just a thought that crossed my mind (don't know the toc code...) Ozgur Ugras BARAN wrote: It is strictly dialog related problem, therefore I should have put some function in controller. The place for a flag like demotionEnabled maybe in TocBackend, but this doesn't remove the necessity of a controller function. IMHO, the code is cleaner as it is now. My question was different, actually. What I am doing now is to prevent demotion from the dialog. Another option would be not to filter TOC entries in TOC dialog, therefore, demotion will never be a problem. The downside of this method is over-crowded TOC dialog. I wonder what people think about current solution? Does anybody have another idea? Ugras On 5/14/07, Edwin Leuven <[EMAIL PROTECTED]> wrote: shouldn't the proper "enabled" flag be set in the kernel (bufferview.cpp?) instead of putting this in the controller? Ozgur Ugras BARAN wrote: > When demote button is pressed too many times, TOC item dissapears from > the toc dialog, since it is no longer numbered. Hence, promoting back > to the original state from TOC dialog is not possible. Attached patch > is a way of correcting this behaviour. It simply prevents demoting the > item more than document default max toc depth. > > What is your idea about the patch? > > regards > > Ugras >
Re: [patch] do not allow too much demote in TOC dialog
On 5/14/07, Andre Poenitz <[EMAIL PROTECTED]> wrote:
On Mon, May 14, 2007 at 08:19:59PM +0200, Ozgur Ugras BARAN wrote:
> +bool const ControlToc::allowDemoteCurrentItem(size_t type) const
> +{
> + return ((kernel().buffer().params().tocdepth -
(*getCurrentTocItem(type)).depth() + 1)>0);
> +}
On the formal side:
No need for the outer parantheses.
removed
I moreover personally find a + 1 > b easier to read then ((a - (b) +
1)>0)
I guess I prefer (a - b +1) > 0. (paranthesis around
(*getCurrentTocItem(type)) is necessary here)
There is no need for the 'const' on the return value either.
removed
Ander'
strange.. nobody complained the code below..
+
moveInTB->setEnabled(form_->allowDemoteCurrentItem(typeCO->currentIndex())
&&
+ moveOutTB->isEnabled()); //means controls are enabled.
regards
Ugras
Index: frontends/qt4/TocWidget.cpp
===
--- frontends/qt4/TocWidget.cpp (revision 18311)
+++ frontends/qt4/TocWidget.cpp (working copy)
@@ -211,6 +211,8 @@
tocTV->selectionModel()->setCurrentIndex(index,
QItemSelectionModel::ClearAndSelect);
tocTV->selectionModel()->blockSignals(false);
+ moveInTB->setEnabled(form_->allowDemoteCurrentItem(typeCO->currentIndex()) &&
+ moveOutTB->isEnabled()); //means controls are enabled.
}
@@ -223,7 +225,8 @@
moveUpTB->setEnabled(enable);
moveDownTB->setEnabled(enable);
- moveInTB->setEnabled(enable);
+ moveInTB->setEnabled(enable &&
+ form_->allowDemoteCurrentItem(typeCO->currentIndex()));
moveOutTB->setEnabled(enable);
depthSL->setEnabled(enable);
Index: frontends/controllers/ControlToc.cpp
===
--- frontends/controllers/ControlToc.cpp (revision 18311)
+++ frontends/controllers/ControlToc.cpp (working copy)
@@ -142,5 +142,10 @@
return _(type);
}
+bool ControlToc::allowDemoteCurrentItem(size_t type) const
+{
+ return (kernel().buffer().params().tocdepth - (*getCurrentTocItem(type)).depth() + 1)>0;
+}
+
} // namespace frontend
} // namespace lyx
Index: frontends/controllers/ControlToc.h
===
--- frontends/controllers/ControlToc.h (revision 18311)
+++ frontends/controllers/ControlToc.h (working copy)
@@ -62,6 +62,8 @@
bool canOutline(size_t type) const;
///
void updateBackend();
+ /// Is current item depth is equal the max allowed toc depth
+ bool allowDemoteCurrentItem(size_t type_) const;
private:
/// Return the guiname from a given cmdName of the TOC param
Re: [patch] do not allow too much demote in TOC dialog
On Mon, May 14, 2007 at 09:47:11PM +0200, Ozgur Ugras BARAN wrote:
> On 5/14/07, Andre Poenitz <[EMAIL PROTECTED]> wrote:
> >On Mon, May 14, 2007 at 08:19:59PM +0200, Ozgur Ugras BARAN wrote:
> >> +bool const ControlToc::allowDemoteCurrentItem(size_t type) const
> >> +{
> >> + return ((kernel().buffer().params().tocdepth -
> >(*getCurrentTocItem(type)).depth() + 1)>0);
> >> +}
> >
> >On the formal side:
> >
> >
> >No need for the outer parantheses.
> >
> removed
>
> >I moreover personally find a + 1 > b easier to read then ((a - (b) +
> >1)>0)
> >
> I guess I prefer (a - b +1) > 0. (paranthesis around
> (*getCurrentTocItem(type)) is necessary here)
But only because you do not write
getCurrentTocItem(type)->depth()
Andre'
