Re: [Patch] Fix bug 1746: table dialog shows wrong settings when LaTeX-argument is entered

2005-02-23 Thread Georg Baum
Martin Vermeer wrote:

> On Tue, 2005-02-22 at 22:37, Georg Baum wrote:
>> Nice idea, but IMHO impossible to implement correctly in finite time with
>> a finite amount of code, because the special string is LaTeX and LaTeX is
>> a mess.
>> 
>> 
>> Georg
> 
> For some definitions of "correctly", yes. I wouldn't go that far. It is
> still the user's reponsibility to write correct LaTeX.

Of course. But even "correct LaTeX" can be horrible. And if we do that, we
should either understand "correct LaTeX" or don't go that way at all. We
could reuse the tex2lyx table column parser for that, but it is still too
much effort IMO.


Georg



Re: [Patch] Fix bug 1746: table dialog shows wrong settings when LaTeX-argument is entered

2005-02-22 Thread Martin Vermeer
On Tue, 2005-02-22 at 22:37, Georg Baum wrote:
> Am Dienstag, 22. Februar 2005 14:52 schrieb Martin Vermeer:
> > So, the default would be centered alignment on the pulldown, and the
> > latex string "c". Then you could add ">{\columncolor{yellow}}" to the
> > left of this c, and if you then choose left alignment, the string will
> > change to  ">{\columncolor{yellow}}l". Or if you replace the c in the
> > string by an r, the pulldown would "sense" it and change to "Right". Or
> > a p{...} and change it to "Block" (and adapt the vertical alignment
> > too).
> > 
> > Tricky to implement, but very intuitive.
> 
> Nice idea, but IMHO impossible to implement correctly in finite time with 
> a finite amount of code, because the special string is LaTeX and LaTeX is 
> a mess.
> 
> 
> Georg

For some definitions of "correctly", yes. I wouldn't go that far. It is
still the user's reponsibility to write correct LaTeX.

- Martin



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


Re: [Patch] Fix bug 1746: table dialog shows wrong settings when LaTeX-argument is entered

2005-02-22 Thread Georg Baum
Am Dienstag, 22. Februar 2005 14:52 schrieb Martin Vermeer:
> So, the default would be centered alignment on the pulldown, and the
> latex string "c". Then you could add ">{\columncolor{yellow}}" to the
> left of this c, and if you then choose left alignment, the string will
> change to  ">{\columncolor{yellow}}l". Or if you replace the c in the
> string by an r, the pulldown would "sense" it and change to "Right". Or
> a p{...} and change it to "Block" (and adapt the vertical alignment
> too).
> 
> Tricky to implement, but very intuitive.

Nice idea, but IMHO impossible to implement correctly in finite time with 
a finite amount of code, because the special string is LaTeX and LaTeX is 
a mess.


Georg



Re: [Patch] Fix bug 1746: table dialog shows wrong settings when LaTeX-argument is entered

2005-02-22 Thread Martin Vermeer
On Fri, 2005-02-18 at 17:24, Georg Baum wrote:
> Martin Vermeer wrote:

...

> I think that we should simply disable the other input boxes for now if
> special is not empty but not change the semantics of the different
> settings.

Actually I think we should make it so, that the latex string box and the
alignment pulldowns are always consistent. A bit like the alignment
characters string | characters and "lines" parameters in the math array
dialog.

So, the default would be centered alignment on the pulldown, and the
latex string "c". Then you could add ">{\columncolor{yellow}}" to the
left of this c, and if you then choose left alignment, the string will
change to  ">{\columncolor{yellow}}l". Or if you replace the c in the
string by an r, the pulldown would "sense" it and change to "Right". Or
a p{...} and change it to "Block" (and adapt the vertical alignment
too).

Tricky to implement, but very intuitive.

- Martin



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


Re: [Patch] Fix bug 1746: table dialog shows wrong settings when LaTeX-argument is entered

2005-02-20 Thread Georg Baum
Am Samstag, 19. Februar 2005 11:59 schrieb Martin Vermeer:
> This | "sensing" in the latex string is really ugly... and here the user
> has the legal recourse of switching the border on or off in the Borders
> tab, no need to make the latex string do that. That's what I meant by
> ugly special coding.

I agree. I am not sure wether there is a case where you need to include 
the "|" in the special string, but if that is needed it is still possible 
to switch the borders of.

> As for lcr(pmb), I suppose that has to stay as it is: the latex string
> is expected to provide the alignment char if it (the string) is non-
> empty. But LaTeX users are on their own anyway, and in the LaTeX
> Companion page 105 table 5.1 an actual example is given how to include
> these alignment chars with a >command or  live with that.

I agree.

> Yes. Actually what would happen (for the present patch) is that some
> vertical lines will be doubled up if provided both through the halign
> pulldown and the latex string. But yes, a conversion would be nice to
> have.

IMO not "would be nice to have", but "must have". If a true conversion is 
too complicated, we should still bump up the version number and provide a 
warning that one has to check manually (this is easy to implement), but 
we should not do this change silently.

> Shall I put a version of my patch in bugzilla "for future reference"?

Maybe simply add a link to this thread in the mailing list archive?


Georg



Re: [Patch] Fix bug 1746: table dialog shows wrong settings when LaTeX-argument is entered

2005-02-19 Thread Martin Vermeer
On Fri, 2005-02-18 at 16:24 +0100, Georg Baum wrote:
> Martin Vermeer wrote:
> 
> > See attached. As a bonus it removes an ugly piece of special coding.
> > 
> > Georg, are you working on this? I remember seeing a post to that effect
> > but cannot find it.
> 
> No, I am not working on this. Maybe you meant the tabular support in
> tex2lyx?

Yes, I mixed that up.

> The table UI is horrible, both in 1.3 and 1.4. A good redesign will change
> the file format and supported features and is not a trivial task, so now is
> not the right time for doing that.

No... and this is not even a regression. The bug (and a UI bug it is!)
is alive and well in 1.3.5 too.

> I am not sure wether I understand the patch (did not try it), 

Attached a better patch, which I made whitespace-agnostic so you see
better what really changes.

> but it looks
> like you output the lines and alignment setting always. IMHO the current
> behaviour (use special and only special if it is not empty) is better,
> because it is not possible to use user defined columns with your change.

Ah yes... but that applies to lcr only, not to |.

This | "sensing" in the latex string is really ugly... and here the user
has the legal recourse of switching the border on or off in the Borders
tab, no need to make the latex string do that. That's what I meant by
ugly special coding.

As for lcr(pmb), I suppose that has to stay as it is: the latex string
is expected to provide the alignment char if it (the string) is non-
empty. But LaTeX users are on their own anyway, and in the LaTeX
Companion page 105 table 5.1 an actual example is given how to include
these alignment chars with a >command or  Also this is a file format change, so if it is going to be applied we need
> a conversion in lyx2lyx, too.

Yes. Actually what would happen (for the present patch) is that some
vertical lines will be doubled up if provided both through the halign
pulldown and the latex string. But yes, a conversion would be nice to
have.

> I think that we should simply disable the other input boxes for now if
> special is not empty but not change the semantics of the different
> settings.

Not even that for 1.4.0. We are in hard freeze and this is not a
crashing bug or regression.

> Georg

Shall I put a version of my patch in bugzilla "for future reference"?

- Martin

Index: tabular.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/tabular.C,v
retrieving revision 1.221
diff -u -w -r1.221 tabular.C
--- tabular.C	27 Jan 2005 21:05:33 -	1.221
+++ tabular.C	19 Feb 2005 10:46:15 -
@@ -626,13 +626,9 @@
 	if (!onlycolumn && isMultiColumn(cell) &&
 		(isFirstCellInRow(cell) || isMultiColumn(cell-1)))
 	{
-		if (cellinfo_of_cell(cell).align_special.empty())
 			return cellinfo_of_cell(cell).left_line;
-		return prefixIs(ltrim(cellinfo_of_cell(cell).align_special), "|");
 	}
-	if (column_info[column_of_cell(cell)].align_special.empty())
 		return column_info[column_of_cell(cell)].left_line;
-	return prefixIs(ltrim(column_info[column_of_cell(cell)].align_special), "|");
 }
 
 
@@ -641,13 +637,9 @@
 	if (!onlycolumn && isMultiColumn(cell) &&
 		(isLastCellInRow(cell) || isMultiColumn(cell + 1)))
 	{
-		if (cellinfo_of_cell(cell).align_special.empty())
 			return cellinfo_of_cell(cell).right_line;
-		return suffixIs(rtrim(cellinfo_of_cell(cell).align_special), "|");
 	}
-	if (column_info[column_of_cell(cell)].align_special.empty())
 		return column_info[right_column_of_cell(cell)].right_line;
-	return suffixIs(rtrim(column_info[column_of_cell(cell)].align_special), "|");
 }
 
 
@@ -1759,9 +1751,6 @@
 	}
 	if (isMultiColumn(cell)) {
 		os << "\\multicolumn{" << cells_in_multicolumn(cell) << "}{";
-		if (!cellinfo_of_cell(cell).align_special.empty()) {
-			os << cellinfo_of_cell(cell).align_special << "}{";
-		} else {
 			if (leftLine(cell) &&
 (isFirstCellInRow(cell) ||
  (!isMultiColumn(cell - 1) && !leftLine(cell, true) &&
@@ -1769,6 +1758,9 @@
 			{
 os << '|';
 			}
+		if (!cellinfo_of_cell(cell).align_special.empty()) {
+			os << cellinfo_of_cell(cell).align_special;
+		} else {
 			if (!getPWidth(cell).zero()) {
 switch (getVAlignment(cell)) {
 case LYX_VALIGN_TOP:
@@ -1797,6 +1789,7 @@
 	break;
 }
 			}
+		}
 			if (rightLine(cell))
 os << '|';
 			if (((cell + 1) < numberofcells) && !isFirstCellInRow(cell+1) &&
@@ -1804,7 +1797,6 @@
 os << '|';
 			os << "}{";
 		}
-	}
 	if (getUsebox(cell) == BOX_PARBOX) {
 		os << "\\parbox[";
 		switch (getVAlignment(cell)) {
@@ -2016,11 +2008,11 @@
 	else
 		os << "\\begin{tabular}{";
 	for (col_type i = 0; i < columns_; ++i) {
+		if (column_info[i].left_line)
+			os << '|';
 		if (!column_info[i].align_special.empty()) {
 			os << column_info[i].align_special;
 		} else {
-			if (column_info[i].left_line)
-os << '|';
 			if (!column_info[i].p_width.zero()) {
 switch (column_info[i].alignment) {
 case LYX_ALIGN_LEFT:
@@ -2066,

Re: [Patch] Fix bug 1746: table dialog shows wrong settings when LaTeX-argument is entered

2005-02-18 Thread Georg Baum
Martin Vermeer wrote:

> See attached. As a bonus it removes an ugly piece of special coding.
> 
> Georg, are you working on this? I remember seeing a post to that effect
> but cannot find it.

No, I am not working on this. Maybe you meant the tabular support in
tex2lyx?
The table UI is horrible, both in 1.3 and 1.4. A good redesign will change
the file format and supported features and is not a trivial task, so now is
not the right time for doing that.

I am not sure wether I understand the patch (did not try it), but it looks
like you output the lines and alignment setting always. IMHO the current
behaviour (use special and only special if it is not empty) is better,
because it is not possible to use user defined columns with your change.
Also this is a file format change, so if it is going to be applied we need
a conversion in lyx2lyx, too.

I think that we should simply disable the other input boxes for now if
special is not empty but not change the semantics of the different
settings.


Georg




[Patch] Fix bug 1746: table dialog shows wrong settings when LaTeX-argument is entered

2005-02-18 Thread Martin Vermeer
See attached. As a bonus it removes an ugly piece of special coding.

Georg, are you working on this? I remember seeing a post to that effect
but cannot find it.

- Martin

Index: tabular.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/tabular.C,v
retrieving revision 1.221
diff -u -r1.221 tabular.C
--- tabular.C	27 Jan 2005 21:05:33 -	1.221
+++ tabular.C	18 Feb 2005 14:26:23 -
@@ -625,14 +625,8 @@
 {
 	if (!onlycolumn && isMultiColumn(cell) &&
 		(isFirstCellInRow(cell) || isMultiColumn(cell-1)))
-	{
-		if (cellinfo_of_cell(cell).align_special.empty())
-			return cellinfo_of_cell(cell).left_line;
-		return prefixIs(ltrim(cellinfo_of_cell(cell).align_special), "|");
-	}
-	if (column_info[column_of_cell(cell)].align_special.empty())
-		return column_info[column_of_cell(cell)].left_line;
-	return prefixIs(ltrim(column_info[column_of_cell(cell)].align_special), "|");
+		return cellinfo_of_cell(cell).left_line;
+	return column_info[column_of_cell(cell)].left_line;
 }
 
 
@@ -640,14 +634,8 @@
 {
 	if (!onlycolumn && isMultiColumn(cell) &&
 		(isLastCellInRow(cell) || isMultiColumn(cell + 1)))
-	{
-		if (cellinfo_of_cell(cell).align_special.empty())
-			return cellinfo_of_cell(cell).right_line;
-		return suffixIs(rtrim(cellinfo_of_cell(cell).align_special), "|");
-	}
-	if (column_info[column_of_cell(cell)].align_special.empty())
-		return column_info[right_column_of_cell(cell)].right_line;
-	return suffixIs(rtrim(column_info[column_of_cell(cell)].align_special), "|");
+		return cellinfo_of_cell(cell).right_line;
+	return column_info[right_column_of_cell(cell)].right_line;
 }
 
 
@@ -2016,59 +2004,59 @@
 	else
 		os << "\\begin{tabular}{";
 	for (col_type i = 0; i < columns_; ++i) {
-		if (!column_info[i].align_special.empty()) {
-			os << column_info[i].align_special;
-		} else {
-			if (column_info[i].left_line)
-os << '|';
-			if (!column_info[i].p_width.zero()) {
-switch (column_info[i].alignment) {
-case LYX_ALIGN_LEFT:
-	os << ">{\\raggedright}";
-	break;
-case LYX_ALIGN_RIGHT:
-	os << ">{\\raggedleft}";
-	break;
-case LYX_ALIGN_CENTER:
-	os << ">{\\centering}";
-	break;
-case LYX_ALIGN_NONE:
-case LYX_ALIGN_BLOCK:
-case LYX_ALIGN_LAYOUT:
-case LYX_ALIGN_SPECIAL:
-	break;
-}
+		if (column_info[i].left_line)
+			os << '|';
+		if (!column_info[i].p_width.zero()) {
+			switch (column_info[i].alignment) {
+			case LYX_ALIGN_LEFT:
+os << ">{\\raggedright}";
+break;
+			case LYX_ALIGN_RIGHT:
+os << ">{\\raggedleft}";
+break;
+			case LYX_ALIGN_CENTER:
+os << ">{\\centering}";
+break;
+			case LYX_ALIGN_NONE:
+			case LYX_ALIGN_BLOCK:
+			case LYX_ALIGN_LAYOUT:
+			case LYX_ALIGN_SPECIAL:
+break;
+			}
 
-switch (column_info[i].valignment) {
-case LYX_VALIGN_TOP:
-	os << 'p';
-	break;
-case LYX_VALIGN_MIDDLE:
-	os << 'm';
-	break;
-case LYX_VALIGN_BOTTOM:
-	os << 'b';
-	break;
+			switch (column_info[i].valignment) {
+			case LYX_VALIGN_TOP:
+os << 'p';
+break;
+			case LYX_VALIGN_MIDDLE:
+os << 'm';
+break;
+			case LYX_VALIGN_BOTTOM:
+os << 'b';
+break;
+		}
+			os << '{'
+			   << column_info[i].p_width.asLatexString()
+			   << '}';
+		} else {
+			// Latex string *in addition* to |, lcr:
+			if (!column_info[i].align_special.empty()) {
+os << column_info[i].align_special;
 			}
-os << '{'
-   << column_info[i].p_width.asLatexString()
-   << '}';
-			} else {
-switch (column_info[i].alignment) {
-case LYX_ALIGN_LEFT:
-	os << 'l';
-	break;
-case LYX_ALIGN_RIGHT:
-	os << 'r';
-	break;
-default:
-	os << 'c';
-	break;
-}
+			switch (column_info[i].alignment) {
+			case LYX_ALIGN_LEFT:
+os << 'l';
+break;
+			case LYX_ALIGN_RIGHT:
+os << 'r';
+break;
+			default:
+os << 'c';
+break;
 			}
-			if (column_info[i].right_line)
-os << '|';
 		}
+		if (column_info[i].right_line)
+			os << '|';
 	}
 	os << "}\n";
 	++ret;


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