Integrate font objects copy/paste with system clipboard
AbandonedPublic

Authored by Dalai Felinto (dfelinto) on Feb 11 2016, 7:42 PM.

Details

Summary

When pasting, the style (bold, material, ...) is maintained, if it was originally copied from Blender.

@Brecht Van Lommel (brecht), @Julian Eisel (Severin) told me that you had thought ideas for this functionality. Let me know if it is along those lines.

TODO: update Blender manual once this get merged

Diff Detail

Repository
rB Blender
Branch
arcpatch-D1789_1

It is worthy noticing that FONT_OT_text_paste_from_clipboard was working, but it doesn't have a key assigned to it.

That said, I strongly believe we should just go ahead with this patch, to allow for intuitive, easy to use copy/paste functionality.

  • Fixup for utf8 strings

For the records, there is still something going on when I start using utf8 and styles (e.g., bold, underline, ...). The issue is present with the copy operator. Maybe something @Campbell Barton (campbellbarton) can help with, during review?

Brecht Van Lommel (brecht) requested changes to this revision.Feb 11 2016, 9:19 PM

The overall approach seems ok, it's what I talked about with @Julian Eisel (Severin) some time ago.

source/blender/blenkernel/BKE_global.h
96–97 ↗(On Diff #6047)

Can we get rid of the fixed copy buffer size and make it dynamic? It seems like a waste of memory to have this always, and an unnecessary limitation.

The variable names can also be more descriptive, copybuf doesn't say much.

source/blender/editors/curve/editfont.c
701–702

Can you move these inside the if(), and free buf in there too instead of setting it to NULL?

We've also got C99 now so no need to define things at the start of the function.

716

Check for malloc failure.

Also not sure why you use len_utf8 + sizeof(wchar_t) here, just len_utf8 + 1 should be enough?

814

I don't think an error needs to be reported for this, all apps I know just silently ignore this.

819

Same comment as above.

This revision now requires changes to proceed.Feb 11 2016, 9:19 PM
  • From review: general style touch ups
  • From review: make buffers dynamic and rename
  • Small code cleanup
Campbell Barton (campbellbarton) requested changes to this revision.Feb 12 2016, 5:29 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/blenkernel/BKE_global.h
94–97 ↗(On Diff #6052)

Would rather not store clipboard here (blender has many clipboards, and seems a bit arbitrary to store this particular clipboard in 'Global').

Conventions from BKE_mask_clipboard_*, BKE_tracking_clipboard_* can work here.

static structs can be access by function if its needed.

BKE_vfont_clipboard_get(wchar_t *r_buf, int *r_buf_len)
source/blender/editors/curve/editfont.c
798–812

Variables could be made more clear, (strp, buf, len)...

While not really confusing, would rather make it more clear whats going on with taking buffers from different sources and comparing.

eg:

/* Store both clipboards as utf8 for comparison,
 * Give priority to the internal 'vfont' clipboard with its 'CharInfo' text styles
 * as long as its synchronized with the systems clipboard. */
struct {
    char *buf;
    int len;
} clipboard_system = {NULL}, clipboard_vfont =  {NULL};

... so logic for checks reads more straightforward.

This revision now requires changes to proceed.Feb 12 2016, 5:29 AM
  • From review: move statics from global and other changes
source/blender/blenkernel/intern/blender.c
117

Any reason this isn't with the other free functions in WM_exit_ext ?

Minor edits

  • use single struct for clipboard
  • store the wchar_t length of the clipboard
  • rename BKE_vfont_clipboard_clear -> BKE_vfont_clipboard_free (more consistent)
  • call free from WM_exit_ext as with other clipboard free functions.
  • use MEM_SAFE_FREE macros

commandeering back so I can close it

Patch commited on rBa143aea - Thanks everyone for the reviews, and patches