STORM-1459 "Wearing Tab" - Add ability to copy displayed inventory names to clipboard | Review Request


Submitter:	Jonathan Yap	Reviewers
Branch:	2.7	Groups:	viewer
Bugs:	STORM-1459	People:
Change Number:	None	Repository:	viewer-development

Description:

Add a feature on the "Wearing TAB" where users could copy to the clipboard everything you see in the "Wearing TAB". This would make the blogging communities life soooooo much easier instead of having to type out all that information.

The label on this button needs input from someone on the XD team.

I would like to know if my code for adding a CR to the end of every line but the last one could be done in a more elegant way.

Testing Done:

Clicked on Send to Clipboard button and was able to paste results into an editor.

Expand All

Looks good apart from missing NULL check.

indra/newview/llpanelwearing.cpp (Diff revision 1)

I'd use const_iterator instead.

indra/newview/llpanelwearing.cpp (Diff revision 1)

A bit of code duplication.
I'd rewrite it like this:

/////////////////////////////////
if (!need_cr)
{
     need_cr = true;
}
else
{
     text += "\n";
}

text += item->getName();
/////////////////////////////////

Please also make sure the item is not NULL to prevent a crash.

Diff
- added Diff r2

Description:

Made changes requested by Oz and Vadim and with help from Alexandrea Fride.

\n is a new line character(line-feed) \r is a carriage return. maybe change the comment to new line.

Diff
- added Diff r3

Description:

Updated comment per Nicky's suggestion

No major objections.

indra/newview/llpanelwearing.cpp (Diff revision 3)

Looks a bit confusing.

Lance Corrimal July 7, 2011, 7:54 a.m. (July 7, 2011, 7:54 a.m.)
```
but it's elegant!
```

Cron Stardust July 7, 2011, 8:50 a.m. (July 7, 2011, 8:50 a.m.)

Just needs some parens:
text += (iter != data.end()) ? item->getName() + "\n" : item->getName();

But otherwise it looks just like a a lot of lines I've run across in the past, and with the comment it's not all that difficult to understand.

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

This change has been marked as submitted.

STORM-1459 "Wearing Tab" - Add ability to copy displayed inventory names to clipboard