Finally...
> |+void Buffer::getSourceCode(ostream& os, lyx::pit_type par_begin,
> lyx::pit_type par_end)
> |+{
>
> space before (and after) '&' please (same for '*')
> (I see that you are not quite consistent)
Yes. Have double-checked, but ...
> |+ // this is personal. I think source code should be in fixed-size font
> |+ QFont font("Courier New");
>
> Perhaps, but the users should be able to decide for itself.
Flexibility is good, but we should not have everything variable as a
rc entry. I will wait till someone complains about the fix font here.
The best thing would be a right button mouse event to change the font.
I only see copy and select though. (Maybe qt4 has more?)
> |+ std::swap(par_begin, par_end);
> |+ ostringstream ostr;
> |+ bv->buffer()->getSourceCode(ostr, par_begin, par_end
> + 1);
> |+ // display the dialog and show source code
> |+ bv->owner()->getDialogs().update("view-source",
> ostr.str());
> |+ }
> | break;
> I don't think it should be the cores responisibility to feed the
> view-source to the frontend, but rather the frontend should come get
> it when needed.
Define 'when needed' please. I intentionally avoid some update button
on the dialog. A user can close the dialog when it is not needed. If
it is open, it will be automatically updated.
> The core should only inform the frontend about a
> cursor change/paragraph change imho.
You are right, To make sure: do you mean to move this pit_begin,
pit_end part to controll/ControllViewSource.C 's update function?
> And it seems you are doing the
>
> |+ if (par_begin > par_end)
> |+ std::swap(par_begin, par_end);
>
> double up? (didn't getSourceCode also do that game?)
No. It has to be done here since getSourceCode will get the [ ) style interval.
> | ParagraphList::const_iterator pit = paragraphs.begin();
> | ParagraphList::const_iterator pend = paragraphs.end();
> |+
> |+ BOOST_ASSERT(runparams.par_begin <= runparams.par_end);
> |+ // if only part of the paragraphs will be outputed
> |+ if (runparams.par_begin != runparams.par_end) {
> |+ pit = boost::next(paragraphs.begin(),
> | runparams.par_begin);
> |+ pend = boost::next(paragraphs.begin(),
> | runparams.par_end);
> |+ // runparams will be passed to nested paragraphs, so
> |+ // we have to reset the range parameters.
> |+ const_cast<OutputParams&>(runparams).par_begin = 0;
> |+ const_cast<OutputParams&>(runparams).par_end = 0;
> |+ }
> |+
> | for (; pit != pend; ++pit) {
> | LyXLayout_ptr const & style = pit->layout();
> | // treat <toc> as a special case for compatibility with old
> code
>
> This also feels wrong. pit and pend should not be modified, but the
> whole output function (perhapa a new one is needed?) should take start
> par and end par.
You mean runparam.par_begin, par_end, right? runparams will be passed
to nested paragraphs and they are supposed to generate the whole
output. This is why I change par_begin, par_end.
> And with those cleaned up I am not sure if you need the change to
> outparams/runparsms anymroe.
Passing par_begin and par_end to every such functions as parameters is
OK, and we can get rid of the runparameter/par_begin stuff. However,
that method is much more intrusive and error prone. (All function
definitions should be changed, and there are some for-loops that need
the right delimiters... etc). Anyway, if you insist, I will change the
implementation. Please re-consider and confirm what you think about
this.
I have submitted the patch. I will make all the changes in a followup
patch, after I hear more from others. (BTW, with all the name changes,
none of my patches can be cleanly applied now. So, better apply
earlier than later. :-)
Cheers,
Bo
