Bug 691572

Created attachment 427858 [details]
pkg desc

BTW, we discussed this issue on 2010-09-10 on the opensuse-packaging ML...

License agreements are no better.  I'll attache the flash license.

Created attachment 429590 [details]
flash license

Starts with tags.

Overlong lines.

Ok, is this a plea for shorter texts?

No, long texts per se a ok.

Long lines are highly problematic.  I'm not sure whether yast can work around this or whether the texts (licenses and package descriptions) need special formatting.

E.g., often the legacy "Authors" lists run in just one line or a paragraph without line-breaks.  Also it is unclear whether some kind of lists or '''inlince''' formatting is still supported.

For reference, see http://pdb.suse.de/doc/user/descriptions.html#pdb_attributes_descriptions_formatting

==============================================================

And the flash license starts with "<small><tt>", which is wrong.

looks like an ui thing

That's yast2-ncurses. Gabi, could you please look into it?

No, other UIs are also affected (see the gtk attachment).

In the end, this is not that much about the yast side, it is more about providing good data that yast then can display nicely.

It is up to the yast team to say how the patch description must be formatted, and then you must insist on using this specification.

In the past, we had it in the PDB--see comment5.

Created attachment 443814 [details]
patch description, gtk ui

The problem with "<small><tt>" tags is reported in bnc #605113. The tags should be removed in PDB.

I am not sure who is responsible for formatting the patch descriptions nicely. Thomas, what do you think?

(In reply to comment #10)
> The problem with "<small><tt>" tags is reported in bnc #605113. The tags should
> be removed in PDB.
> 
> I am not sure who is responsible for formatting the patch descriptions nicely.
> Thomas, what do you think?

I talked to Marcus Meißner. Patch descriptions are formatted as plain text. I guess the tags make the package selector believe it's rich text. Which leads to ignored line breaks. Removing these tags as requested in bug 605113 could solve the line break issue as a side effect.

No, these "<small><tt>" tags are not the cause of the problem.  See the kernel patch description: https://bugzilla.novell.com/attachment.cgi?id=443814

The cause probably is that yast wants description as documented in http://pdb.suse.de/doc/user/descriptions.html#pdb_attributes_descriptions_formatting --which I already referenced in comment 5 ;)

Let's start over again.  Nothing has changed for good.

First we please must decide whether http://pdb.suse.de/doc/user/descriptions.html#pdb_attributes_descriptions_formatting is still considered to be valid.

If yes, we must communicate this to the yast developers, text writers, patch description writers, etc.  Note, this is not a yast-ncurses issue!

I'll attach another ugly example released the last days (openSUSE-2012-83).

Created attachment 475800 [details]
Ugly formatted description text (gtk UI, the same with ncurses)

I will try to give an overview of the problem(s) here.

There are different descriptions mentioned in this bug which come from different places. And where they come from depends on the product:

           license         package description      patch description
---------------------------------------------------------------------
SLES         pdb                  pdb                   SWAMP

openSUSE     ?                 spec-file                SWAMP

For ncurses I have fixed the bug reported in comment #3 with html tags in flash-player license (available in Factory). The problem was that html tags inside <pre> tags weren't interpreted from text mode.
The problem with "overlong lines" cannot be fixed in YaST because the license is provided in <pre> ... </pre> tags and the package selector has the only possibility to show the text as it is.

What we need are guide lines for the different descriptions. 
The place where this is documented is:

- for openSUSE package description:
  http://en.opensuse.org/openSUSE:Specfile_guidelines#Description

- for openSUSE and SLES patch descriptions:
  SWAMP

- for SLES licenses:
  pdb

- for openSUSE licenses:
  not yet sure where they come from

(In reply to comment #17)
> 
> - for SLES licenses:
- for SLES license AND package description
>   pdb
>

I have found a guide line for package descriptions on 'pdb', 
see http://pdb.suse.de/doc/user/descriptions.html#pdb_attributes_descriptions_formatting

I suggest to take this (resp. parts of it) for the future guide line for package descriptions in openSUSE. The package descriptions which are imported for openSUSE to the spec-files mostly fulfill the guide line.

I have submitted yast2-ncurses-pkg-2.22.1 to Factory which creates html text from the plain package description following this rules:

From 'pdb' guide line:
    empty lines will be <p> tags. In a list environment, empty lines end the list. linebreaks result in a single space, that means that a line break has no special effect.

    a not numbered list environment starts with a line that starts with an asterix or a minus sign followed by at least one whitespace.

I have added:
    a line which continues a list item starts with at least two whitespaces

I have changed the guide lines for package descriptions on pdb
(http://pdb.suse.de/doc/user/descriptions.html#pdb_attributes_descriptions_formatting):

A limitation to 70 characters for the line length (due to limited size of description window in text mode YaST ) is not necessary any longer. Like the Qt front end, also the ncurses package selector creates simple html text from the plain description (available in Factory since March 2012).

Please respect these rules: Empty lines result in paragraphs. In a list environment, empty lines end the list. Linebreaks result in a single space, that means that a line break has no special effect. A not numbered list environment starts with a line that starts with an asterix or a minus sign followed by at least one white space. A line which continues a list item starts with at least two whitespaces.

The Qt package selector is also enhanced to show not numbered lists nicely (empty lines are already interpreted).

(In reply to comment #20)
> I have changed the guide lines for package descriptions on pdb
> (http://pdb.suse.de/doc/user/descriptions.html#pdb_attributes_descriptions_formatting):
> 
> A limitation to 70 characters for the line length (due to limited size of
> description window in text mode YaST ) is not necessary any longer. Like the Qt
> front end, also the ncurses package selector creates simple html text from the
> plain description (available in Factory since March 2012).
> 
> Please respect these rules: Empty lines result in paragraphs. In a list
> environment, empty lines end the list. Linebreaks result in a single space,
> that means that a line break has no special effect. A not numbered list
> environment starts with a line that starts with an asterix or a minus sign
> followed by at least one white space. A line which continues a list item starts
> with at least two whitespaces.
> 
> The Qt package selector is also enhanced to show not numbered lists nicely
> (empty lines are already interpreted).
The Qt part is fixed in yast2-qt-pkg version 2.21.19.

(In reply to comment #20)
> I have changed the guide lines for package descriptions on pdb
> (http://pdb.suse.de/doc/user/descriptions.html#pdb_attributes_descriptions_formatting):

Totally WRONG - I haven't changed anything on pdb but took parts of the pdb guide lines and entered it in openSUSE wiki:

see http://en.opensuse.org/openSUSE:Specfile_guidelines#Description

I suggest to take these guide lines also for the patch descriptions.
They should be added to SWAMP info text for 'DESCRIPTION'.
Thomas (tschmidt and tgoettlicher), do you agree?

(In reply to comment #23)
> I suggest to take these guide lines also for the patch descriptions.

- Empty lines result in paragraphs. In a list environment, empty lines end the 
  list. Linebreaks result in a single space, that means that a line break has no 
  special effect.
- A not numbered list environment starts with a line that starts with an asterix 
  or a minus sign followed by at least one white space.
- A line which continues a list item starts with at least two whitespaces.

> They should be added to SWAMP info text for 'DESCRIPTION'.
> Thomas (tschmidt and tgoettlicher), do you agree?

Added maint-coord@suse.de to CC.

Do you agree to add rules from comment #24 to SWAMP info for the patch descriptions?

Last point here is about the licenses.

The current behaviour of the text mode package selector is:

If the text is already html text, display it as RichText (as mentioned in comment #16 the problem with visible <small><tt> tags is solved).

If the text is plain text add <pre> </pre> around to keep the text formatting.

The person who is responsible for adding the license has to take care that the license text is readable, means no overlong lines, no form feeds, only valid chars...

I think I was added by mistake to this bug.

Dirk, what do you think? Does it make sense to use suggested rules (comment #24) for the path descriptions?
Other possibility is to add <pre>...</pre> tags around the description which would preserve the formatting. Disadvantage of this approach is that the line length isn't adapted then to the terminal width like it is done when using paragraphs.

Gabi, do not change the product...

just needinfo maint-coord (SLE) or maintenance@opensuse.org (openSUSE)

It's NOT about yast2-ncurses. It's about formatting in general. We need guide lines for the descriptions and the licenses. The ncurses and qt package selector currently in Factory respect the rules for the descriptions.

If we agree to use suggested rules also for the patch descriptions they have to be added to SWAMP info and product SWAMP should fit then?

We write patchinfo documentation in MarkDown syntax: 
http://daringfireball.net/projects/markdown/

I don't see the point in following self-invented rules. I would prefer to stick with the existing syntax rules.

I agree. But the problem is that the patch description is delivered as plain text from libzypp to the UIs and therefore all formatting gets lost.

The package selector need to have the text formatted in html (with doctype tag "<!-- DT:Rich -->" at the beginnning) to be able to show it correctly.

More exactly the text has to be in RichText format which is also interpreted from text mode YaST. We have to check whether all tags from Markdown syntax are supported.

Does SWAMP already export the text in html format?

Because the build service will be responsible for maintenance in future, the problem with patch descriptions will be there as well. 
I had a short discussion with Michael (mls) and Adrian about it. Michael pointed out that the description should be transfered as is to the update info (without html tags). 
Best place to transform plain text to RichText format is probably libzypp. All UIs would have the description available to be able to show it nicely.

Duncan do you agree to add such a functionality to libzypp?

Created attachment 481574 [details]
Description of YaST ncurses RichText

I have attached a short summary of known html tags in text mode YaST.

I don't think libzypp is the right place for this. But also not the job of the concrete UI implementations.

Giving the fact that our current YRichText::setText() implementation assumes you are giving (simplified) HTML (unless setTextMode() is enabled) I would keep the magic in this area. Therefore libyui currently assumes that a concrete UI Foo implementation is handling HTML in FooRichText.

I would add a C/C++ markdown parser to libyui, and modify setText so that if <!-- DT:Rich --> is found at the beginning then the text is set "as is" (assumed HTML). We can detect it in other ways as well. Also, AFAIK, html is valid markdown, so we could just process the text always.

If not, we assume it is markdown, and process the text. We need to verify that ncurses can cope with the markdown parser HTML output, or at least it can ignore the unknown tags.

If setTextMode is enabled and the text does not contain <!-- DT:Rich --> then we set the text unprocessed.

Now, IIRC, the Qt selector does not use the libyui widget, but the ncurses one does, right?

(In reply to comment #36)
> Now, IIRC, the Qt selector does not use the libyui widget, but the ncurses one
> does, right?
The Qt package selector uses QTextBrowser and does itself some rich text magic.

(In reply to comment #36)
> I don't think libzypp is the right place for this. But also not the job of the
> concrete UI implementations.
> 
> Giving the fact that our current YRichText::setText() implementation assumes
> you are giving (simplified) HTML (unless setTextMode() is enabled) I would keep
> the magic in this area. Therefore libyui currently assumes that a concrete UI
> Foo implementation is handling HTML in FooRichText.
I agree, libyui is the right place to add a markdown parser.
> 
> I would add a C/C++ markdown parser to libyui, and modify setText so that if
> <!-- DT:Rich --> is found at the beginning then the text is set "as is"
> (assumed HTML). We can detect it in other ways as well. Also, AFAIK, html is
> valid markdown, so we could just process the text always.
> 
Currently the <!-- DT:Rich --> tag is not used in YaST (YCP code) to indicate html text, i.e. we have to detect html in another way.
> If not, we assume it is markdown, and process the text. 

The disadvantage of having the conversion in YRichText::setText() is that not all text passed to it IS written in markdown syntax (e.g. licenses).

I propose to add a method createHmtlText() which can be called only if needed, e.g. in ncurses package selector for package and patch descriptions.
Disadvantage of this approach is, of course that's not available from YCP.

> We need to verify that
> ncurses can cope with the markdown parser HTML output, or at least it can
> ignore the unknown tags.
This should be OK.
> 
> If setTextMode is enabled and the text does not contain <!-- DT:Rich --> then
> we set the text unprocessed.
> 
> Now, IIRC, the Qt selector does not use the libyui widget, but the ncurses one
> does, right?

I tried CPP-Markdown from:
http://stackoverflow.com/questions/889434/markdown-implementations-for-c-c

It's under MIT license and works well for me.
The boot regex library has to be linked - I don't know whether we want to do this?

I will attach a diff for libyui to show the usage (it's a diff in openSUSE-12_2 branch).

Created attachment 499862 [details]
YRichText.diff

Created attachment 499863 [details]
cpp-markdown-100.zip

CPP Markdown downloaded from http://cpp-markdown.sourceforge.net

Created attachment 499864 [details]
YRichText.diff

Thomas, do you suggest another (better) way to integrate the markdown parser?
And could you please check whether linking of boost_regex is desired or has any disadvantages.

(In reply to comment #43)
> Thomas, do you suggest another (better) way to integrate the markdown parser?
I like the way how the markdown parser is integrated.

> And could you please check whether linking of boost_regex is desired or has any
> disadvantages.
There are some regexp functions in libzypp that do not need the boost library. Copying these functions to the libyui code would prevent us from adding new library dependencies. Michael Andres can tell you details about these libyzpp regexp functions.

Michel, which files are available, what's the best approach to use it?

You need zypp/base/Regex.h and Regex.cc. It provides:

    class smatch;
    class regex;

They can be used similar to the boost ones. It of course depends on the amount of boost features you are using. Those classes provide just the feature subset zypp is using.

You probably need to remove the 'zyyp' stuff (namespaces, logging, exceptions,..) before you can use it in your code.

12.1 is out of support scope. Please report this on github for libyui https://github.com/libyui