+1 Thanks for making my API shine :) -- Denis Gervalle SOFTEC sa - CEO On mar., août 1, 2017 at 14:59, Eduard Moraru <enygma2002(a)gmail.com> wrote: Forgot to mention that most of the very nice styling/css work was done by Caty :) Thanks, Eduard On Tue, Aug 1, 2017 at 3:57 PM, Eduard Moraru <enygma2002(a)gmail.com> wrote: my by should already

+1 very practical and we can make it even better :) Thanks, Caty On Tue, Aug 1, 2017 at 4:07 PM, Alexandru Cotiuga < alexandru.cotiuga(a)xwiki.com&gt; wrote: > +1 > > Thanks, > Alex > > On Tue, Aug 1, 2017 at 4:05 PM, Denis Gervalle &lt;dgl(a)softec.lu&gt; wrote: > >> +1 Thanks for making my API shine :) >> -- >> Denis Gervalle >> SOFTEC sa - CEO >> On mar., août 1, 2017 at 14:59, Eduard Moraru &lt;enygma2002(a)gmail.com&gt; >> wrote: >> Forgot to mention that most of the very nice styling/css work was done by >> Caty :) >> >> Thanks, >> Eduard >> >> On Tue, Aug 1, 2017 at 3:57 PM, Eduard Moraru &lt;enygma2002(a)gmail.com&gt; >> wrote: >> >>> Hi, devs, >>> >>> During a hackathon session, I have done a refresh on XWiki's code > viewer >>> ("code.vm") and integrated the Blame API [1] developed by Denis to add >>> line-by-line blame information, just like GitHub's blame feature. >>> >>> Please see the associated Jira issue that also includes before and > after >>> screenshots: >>> https://jira.xwiki.org/browse/XWIKI-14578 >>> >>> The Blame API module is a commons module since 2014 but not bundled >>> neither in the WAR nor in the flavor, so, in order to go along and > merge >> my >>> work, I'd need it to be available as a core extension (to be also > usable >> by >>> code.vm). >>> >>> I have not studied it deeply, but the module seems to be doing its job >>> well and the result is very nice and it is very generic. >>> >>> Being able to perform a blame analysis builds upon the diff module and >>> both are features of XWiki's versioning capabilities, so, IMO, both >> should >>> be considered core extensions (and not only the diff module, which >> already >>> is core). >>> >>> The PR is available at https://github.com/xwiki/ > xwiki-platform/pull/605 >>> >>> Here is my (obvious) +1 to bundle the blame-api and merge the PR which >>> includes the UI. >>> >>> Thanks, >>> Eduard >>> >>> ---------- >>> [1] http://extensions.xwiki.org/xwiki/bin/view/Extension/ > Blame%20Module >>> >>

On 1 Aug 2017, at 16:52, Eduard Moraru &lt;enygma2002(a)gmail.com&gt; wrote: Hi, Vincent, It's nothing fancy right now, as you can see from the PR: https://github.com/xwiki/xwiki-platform/pull/605/files#diff-68b60a2e4f24889… Basically there's code.vm that gets the document content and handles a request variable. It either uses the Blame API to produce the annotated line content and build the blame-enabled UI OR it just splits the content by lines and builds the simple UI. I guess it could be done with an UIXP and then code.vm could provide only the simple UI when no UIX is detected or when explicitly asked by the user. If the UIX is detected, then code.vm could show the toggle button and render the UIX instead of the simple UI, when the user asks for it. Then it would be a matter of adding an xwiki-platform-blame-ui module to the standard flavor that implements the UIX and brings in the dependency on the commons-blame-api module, so it wouldn`t have to be a core extension. The CSS would also have to be broken in 2, with some of it going to this blame-ui module. This also results in the conclusion that the blame feature is optional and that it would not be available in an empty wiki, where only the simple view source UI would be available. This refactoring would require about another half a day of work. Will need to see when I could do it. Would you consider the effort worth it? The UIXP itself is not really valuable, the only advantage is in not having to add another core extension.

WDYT? Thanks, Eduard On Tue, Aug 1, 2017 at 5:01 PM, Vincent Massol &lt;vincent(a)massol.net&gt; wrote: > Hi Edy, > > +1 in general. Ideally, even though we should bundle it by default in our > base flavor, it would be nice if it could be optional (i.e. if the blame > API/UI modules are present in the WAR then clicking “view source” would > display the blame view and if not, it would default to basic source > viewing). Is that how you coded it? I’m saying “ideally” since it’s > probably more work to do this (might even possibly require a UIX - or the > whole source VM should be a UIX, I don’t know - BTW that might be cool to > have automatic UIX based on template names to unify skins and UIX but > that’s another topic ;)) and at this stage I’m already very happy that > we’re getting this feature and you may not want to spend more time on it > right now! :) > > Nice work! > > Thanks > -Vincent > >> On 1 Aug 2017, at 14:57, Eduard Moraru &lt;enygma2002(a)gmail.com&gt; wrote: >> >> Hi, devs, >> >> During a hackathon session, I have done a refresh on XWiki's code viewer >> ("code.vm") and integrated the Blame API [1] developed by Denis to add >> line-by-line blame information, just like GitHub's blame feature. >> >> Please see the associated Jira issue that also includes before and after >> screenshots: >> https://jira.xwiki.org/browse/XWIKI-14578 >> >> The Blame API module is a commons module since 2014 but not bundled > neither >> in the WAR nor in the flavor, so, in order to go along and merge my work, >> I'd need it to be available as a core extension (to be also usable by >> code.vm). >> >> I have not studied it deeply, but the module seems to be doing its job > well >> and the result is very nice and it is very generic. >> >> Being able to perform a blame analysis builds upon the diff module and > both >> are features of XWiki's versioning capabilities, so, IMO, both should be >> considered core extensions (and not only the diff module, which already > is >> core). >> >> The PR is available at https://github.com/xwiki/xwiki-platform/pull/605 >> >> Here is my (obvious) +1 to bundle the blame-api and merge the PR which >> includes the UI. >> >> Thanks, >> Eduard >> >> ---------- >> [1] http://extensions.xwiki.org/xwiki/bin/view/Extension/Blame%20Module > >

Hi Edy, Actually I have some additional questions: * How well does it perform? Imagine that there’s a 300 lines document and 300 revisions, each one changing one line from the document. How long does it take to display the blame view? * AFAIK our JRCS API is not very performant and thus I’m worried about performance. * I can imagine 2 things to help with this: ** 1) Do not display the blame view by default when going to the source view (let the user click a “show blame view” button) ** 2) Use a progress bar to compute the blame UI. I hope that the Java API has provision for this. WDYT? Could you share with us some stats on performance? Thanks -Vincent neither well both is