Re: [patch BRANCH] insertInset

2008-03-07 Thread Juergen Spitzmueller
Jean-Marc Lasgouttes wrote:

> Well, the first change is straight forward, 

Right. See attached patch.

> but I do not know what is 
> the right semantics to use for the second problem.

Me neither.

Jürgen
Index: src/Text3.cpp
===
--- src/Text3.cpp	(Revision 23533)
+++ src/Text3.cpp	(Arbeitskopie)
@@ -272,6 +272,8 @@
 		lyx::dispatch(FuncRequest(LFUN_CUT));
 		gotsel = true;
 	}
+	bool const emptypar = cur.lastpos() == 0;
+	pos_type ins_pos = cur.pos();
 	text->insertInset(cur, inset);
 
 	if (edit)
@@ -281,8 +283,8 @@
 		// metrics might be invalid at this point (bug 4502)
 		cur.bv().updateMetrics();
 		lyx::dispatch(FuncRequest(LFUN_PASTE, "0"));
-		
-		if (cur.lastpit() == 0) {
+
+		if ((cur.lastpit() == 0 || ins_pos != 0) && !emptypar) {
 			// reset first par to default
 			Layout_ptr const layout =
 cur.buffer().params().getTextClass().defaultLayout();



Re: [patch BRANCH] insertInset

2008-03-06 Thread Jean-Marc Lasgouttes
Juergen Spitzmueller <[EMAIL PROTECTED]> writes:

> I see. The "good" news is that this was not introduced by my change. I'll
> have a look at both issues, if you do not beat me to it.

Well, the first change is straight forward, but I do not know what is
the right semantics to use for the second problem.

JMarc


Re: [patch BRANCH] insertInset

2008-03-06 Thread Juergen Spitzmueller
Jean-Marc Lasgouttes wrote:

> There is still one case that might matter: when one selects the whole
> contents of one Itemize paragraph, your code does not trigger, and I
> think it should. 

Yes.

> The right test is maybe to check whether the 
> enclosing inset is alone in its paragraph.

Probably, yes.

> If one tries to select the end-of-line marker of the paragraph, it
> gets worse: start from
> * aaa
> * bbb
> select from start of aaa to before start of bbb and insert a note. You
> get:
> 
> Note | * aaa
> | *
> bbb
> 
> The second itemize item is empty, which should of course never happen.

I see. The "good" news is that this was not introduced by my change. I'll
have a look at both issues, if you do not beat me to it.

Jürgen



Re: [patch BRANCH] insertInset

2008-03-06 Thread Jean-Marc Lasgouttes
Juergen Spitzmueller <[EMAIL PROTECTED]> writes:

> I finally got too annoyed by the current behaviour when inserting a
> collapsable over a selection.

You will get a medal for this.

> The new approach is to keep the layout of the inner first paragraph
> and reset the outer paragraph to Standard, if the selection spans
> multiple paragraphs. If not (i.e. for inline selections), the inset
> content is still reset to Standard. This strikes me the correct
> behaviour.

There is still one case that might matter: when one selects the whole
contents of one Itemize paragraph, your code does not trigger, and I
think it should. The right test is maybe to check whether the
enclosing inset is alone in its paragraph.

If one tries to select the end-of-line marker of the paragraph, it
gets worse: start from
  * aaa
  * bbb
select from start of aaa to before start of bbb and insert a note. You
get:

Note | * aaa
 | *
bbb

The second itemize item is empty, which should of course never happen.

JMarc


Re: [patch BRANCH] insertInset

2008-03-06 Thread Martin Vermeer
On Thu, 06 Mar 2008 10:46:29 +
José Matos <[EMAIL PROTECTED]> wrote:

> On Thursday 06 March 2008 10:21:42 Juergen Spitzmueller wrote:
> > OK?
> 
> +1
> 
> > Jürgen

Yes, what took so long to backport it?

Ah... that would have been my job :-(

- Martin


Re: [patch BRANCH] insertInset

2008-03-06 Thread José Matos
On Thursday 06 March 2008 10:21:42 Juergen Spitzmueller wrote:
> OK?

+1

> Jürgen

-- 
José Abílio


[patch BRANCH] insertInset

2008-03-06 Thread Juergen Spitzmueller
I finally got too annoyed by the current behaviour when inserting a
collapsable over a selection.

As it is now, when you select some paragraphs from an itemize list and
insert a branch, the first paragraph is reset to Standard, and the branch
itself is in an itemize list. The new approach is to keep the layout of the
inner first paragraph and reset the outer paragraph to Standard, if the
selection spans multiple paragraphs. If not (i.e. for inline selections),
the inset content is still reset to Standard. This strikes me the correct
behaviour.

We already have this new behaviour in trunk, so this is basically a
backport.

OK?

Jürgen
Index: src/Text3.cpp
===
--- src/Text3.cpp	(Revision 23464)
+++ src/Text3.cpp	(Arbeitskopie)
@@ -281,13 +281,21 @@
 		// metrics might be invalid at this point (bug 4502)
 		cur.bv().updateMetrics();
 		lyx::dispatch(FuncRequest(LFUN_PASTE, "0"));
-		// reset first par to default
-		if (cur.lastpit() != 0 || cur.lastpos() != 0) {
+		
+		if (cur.lastpit() == 0) {
+			// reset first par to default
 			Layout_ptr const layout =
 cur.buffer().params().getTextClass().defaultLayout();
 			cur.text()->paragraphs().begin()->layout(layout);
+		} else {
+			// reset surrounding par to default
+			docstring const layoutname = 
+cur.buffer().params().getTextClass().defaultLayoutName();
+			cur.leaveInset(*inset);
+			text->setLayout(cur, layoutname);
 		}
 	}
+
 	return true;
 }