Review Request 111264: fix unwanted changing of m_name in DocxXmlStylesReader::read_style()

Friedrich W. H. Kossebau kossebau at kde.org
Sat Jun 29 01:53:28 BST 2013



> On June 28, 2013, 3:56 p.m., Matus Uzak wrote:
> > Hi Friedrich, the m_name variable is not used anywhere else in the class but if it would be used, then it must have the same value as was assigned to the styleName variable.  Both should contain the same style name with spaces replaced by an underscore.

Ah, okay, thanks for the explanation. Shall I add a comment to the line, so the next coming across and wondering can learn why it is that way?


- Friedrich W. H.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111264/#review35240
-----------------------------------------------------------


On June 26, 2013, 9:22 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111264/
> -----------------------------------------------------------
> 
> (Updated June 26, 2013, 9:22 p.m.)
> 
> 
> Review request for Calligra, Inge Wallin and Matus Uzak.
> 
> 
> Description
> -------
> 
> Saw that when going over all the usages of QString::replace(...) (which changes the object itself and returns a reference to it, as you surely know)
> 
> Looks fishy to me that m_name is changed by the call to replace on creating a name for the style. No idea if that is wanted, I rather guess not.
> Matus, Inge, you are more the experts here, so please give it a check :)
> 
> Okay to backport to 2.7 & 2.6 ?
> 
> 
> Diffs
> -----
> 
>   filters/words/docx/import/DocxXmlStylesReader.cpp 8b38716 
> 
> Diff: http://git.reviewboard.kde.org/r/111264/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130629/2967ac07/attachment.htm>


More information about the calligra-devel mailing list