<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/109021/">http://git.reviewboard.kde.org/r/109021/</a>
     </td>
    </tr>
   </table>
   <br />










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On February 19th, 2013, 9:51 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="http://git.reviewboard.kde.org/r/109021/diff/1/?file=114383#file114383line300" style="color: black; font-weight: bold; text-decoration: underline;">conf/dlggeneralbase.ui</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>



 
 

 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">300</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">         <string>%</string></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">290</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">         <string>Defines font for plain text documents.</string></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is not strictly true, it also applies for epub, fictiobook, mobipocket and any other future backend that uses TextDocumentGenerator.

I think it may make more sense if you add it in the "backend specific" configuration pages in addPages() like the spectre backend does.

The naming still needs to be good, not sure i can think of one now. Anyone has ideas?</pre>
 </blockquote>



 <p>On February 22nd, 2013, 9:40 p.m. UTC, <b>Azat Khuzhin</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes we could write "Defines font for text based documents." or something like this.

As for addPages() in which window it adds options?

Also I'm not sure about this, because the text also can be extracted from PDF and use the same font for rendering. No?</pre>
 </blockquote>





 <p>On February 22nd, 2013, 9:41 p.m. UTC, <b>Azat Khuzhin</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Sorry for the delay, forgot to publish</pre>
 </blockquote>





 <p>On February 23rd, 2013, 11:56 a.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">"Yes we could write "Defines font for text based documents." or something like this."
The problem here is, would people consider  DVI or PDF a "text based document" because that font is not going to apply there? Also in epub fonts can specify the font sometimes too, so not sure how to properly handle this. Maybe we should have one entry per generator that is a TextDocumentGenerator? This way it would solve the issue since you'd clearly see for all the formats it applies, but it'd mean you could configure different default fonts for txt and for a dfferent format. Does that make sense?

"As for addPages() in which window it adds options?"
To the "Configure backends" dialog

"Also I'm not sure about this, because the text also can be extracted from PDF and use the same font for rendering. No?"
It *could*, but it isn't nor won't, doesn't make any sense for PDF where the document specifies the font to use</pre>
 </blockquote>





 <p>On February 24th, 2013, 8:43 p.m. UTC, <b>Azat Khuzhin</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Do you think that people looking at "Configure backends" dialog often?
I think that Font settings must be at main dialog, but at hover we must show tooltip "For ... formats". no?

I starting to implement this using addPages() but it is a bit tricky:
- If people must install font for every backend separately it will annoying.
- If we will change all fonts in all backends to installed one, when one font for one backend is changed by user, than it will be also not good.
- And also we can add multiple backends configurations:
// Something like this
addPage(w, TextDocumentsSettings::self(), i18n( "Txt\nEPub\n...." ), "okular-textdocuments", i18n("Text Based Documents Backend Configuration") );

But when you need to add another settings, for Txt backend for example, than we must split this.

What you think about this?</pre>
 </blockquote>





 <p>On February 28th, 2013, 7:41 p.m. UTC, <b>Azat Khuzhin</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Any news?</pre>
 </blockquote>





 <p>On February 28th, 2013, 11:51 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have no clue if people look at Configure backends or not, but adding more and more backend-specific options to the general options is a no go. Look at the new pdf-only setting we introduced about line thinness, would it make any sense in the general config?

What do you mean with "install font for every backend"?

I'm thinking we could provide a method in TextDocumentGenerator that returns a QWidget with the common settings and then each generator that uses TextDocumentGenerator can call that function and add stuff to the widget if needed or just return it if it has no extra settings to configure. The question about if it makes sense to have defaults fonts for each text based backend still stands though.</pre>
 </blockquote>





 <p>On March 1st, 2013, 12:28 a.m. UTC, <b>Eike Hein</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think going for per-backend options might be the most sensible, especially looking toward future extensibility. For example, the epub backend should ideally have many of the options commonly found in ebook readers, which include text formatting options on a per-element basis (headings, paragraphs, block quotes, ..) as well as the ability to ignore the epub's bundled stylesheets partially (to override the font family, paragraph spacing, paragraph indentation, margins and others - many store-bought epubs come with insane defaults).

Users are also likely to want to use a proportional serif font to read prose text / epubs but a monospace font for plain text files, or at least a sans-serif one, so there's a case to make that different file formats have prevalent use cases and it makes sense to divide the font option between file formats.

The alternative would be to somehow tag generators by use case, i.e. group all "ebook formats" together and have settings for them, but it's unclear we can come up with a better classification (where better would be "less edge cases that don't fit", e.g. an epub doesn't _need_ to be a prose text book) than going by file format.

If per-generator options are too messy, then I guess some sort of cascading settings system would be needed where you could set a default font for all generators and then override per-generator, but that invites quite some complexity.</pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Good, so let's go with my "a method in TextDocumentGenerator that returns a QWidget with the common settings and then each generator that uses TextDocumentGenerator can call that function and add stuff to the widget if needed or just return it if it has no extra settings to configure", then</pre>
<br />




<p>- Albert</p>


<br />
<p>On February 23rd, 2013, 11:56 a.m. UTC, Azat Khuzhin wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Okular, Albert Astals Cid and Eike Hein.</div>
<div>By Azat Khuzhin.</div>


<p style="color: grey;"><i>Updated Feb. 23, 2013, 11:56 a.m.</i></p>






<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Development history:
https://github.com/azat/okular/compare/master...font-selector-for-plain-text-formats

Link to thread from mailing list:
http://comments.gmane.org/gmane.comp.kde.devel.okular/13279</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested manually</pre>
  </td>
 </tr>
</table>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>conf/dlggeneralbase.ui <span style="color: grey">(f2c9efd)</span></li>

 <li>conf/okular_core.kcfg <span style="color: grey">(054b5c1)</span></li>

 <li>core/textdocumentgenerator.h <span style="color: grey">(dd75c5c)</span></li>

 <li>core/textdocumentgenerator.cpp <span style="color: grey">(f370ded)</span></li>

 <li>core/textdocumentgenerator_p.h <span style="color: grey">(749d6f2)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/109021/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>