Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UTF-8 support in canvas (panels, graph) #10319

Merged
merged 7 commits into from Jun 12, 2018

Conversation

cyanpencil
Copy link
Contributor

Rewritten how canvas.c works to handle wide utf8 chars without breaking the panels or graph ASCII layout.

Now canvas, instead of having a single long buffer for the contents of the screen, works with a 2d matrix of variable length rows, which are are enlarged or shortened based on how many "hidden" UTF-8 bytes are printed to screen.

Here's a screen of panels view with utf8 lines and some comments in arabic and braille:
2018-06-11-164637_1193x521_scrot

NOTE: some languages (e.g., russian) have some bytes that interfere with the old runes system. If this pr works (and won't break any tests) I'll disable the runes system to use only UTF-8 in a later pr and have global support.



Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty lines

}
return ret;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty line

c->sx = 0;
c->sy = 0;
c->b = malloc (sizeof *c->b * h);
if (!c->b) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe better to use goto to ease all those exit paths

}
int i;
for (i = 0; i < h; i++) {
c->b[i] = malloc (w + 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check if allocation fails

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

um, isn't it done three lines below? (if !c->b[i] ...)

c->h = h;
c->x = c->y = 0;
c->attrslen = 0;
c->attrs = calloc (sizeof (*c->attrs), (c->w + 1) * c->h + 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c->w * c->h can overflow?

if ((c->b[y][x] & 0xc0) != 0x80) {
atr = attr_at (c, y*c->w + attr_x);
if (atr && *atr) {
strcat (o, *atr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strcat already does strlen, so the strlen after that is unnecessary, so you can do strlen + memcpy(or memmove if its in the same buffer) instead

@@ -33,7 +33,7 @@
#define PANEL_CMD_STACK "px 256@r:SP"
#define PANEL_CMD_REGISTERS "dr="
#define PANEL_CMD_REGISTERREFS "drr"
#define PANEL_CMD_DISASSEMBLY "pd $r @e:scr.utf8=0"
#define PANEL_CMD_DISASSEMBLY "pd $r"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

@@ -606,6 +606,7 @@ static void panelsRefresh(RCore *core) {
}
if (panels->isResizing) {
panels->isResizing = false;
/*eprintf ("REEEEEEEEEEEEEESSSSS\n");*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm this

@radare
Copy link
Collaborator

radare commented Jun 11, 2018

awesome work!

@cyanpencil
Copy link
Contributor Author

Thank you for the useful revisions.

I tried to do the error handling with goto, tell me if I did it correctly...

free (c->b[j]);
}
free (c->bsize);
goto error_bsize;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary goto

}
error_bsize:
free (c->blen);
goto error_blen;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary goto

goto error_blen;
error_blen:
free (c->b);
goto error_b;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary goto

c->attr = Color_RESET;
r_cons_canvas_clear (c);
return c;
error_c: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as long as its initialized to 0, you can safely call free on all the fields without having to add many labels and gotos. just 1

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in gstreamer.. and eventually in r2land, we name those labels as locations. Like beach. so you can goto beach;

@radare radare merged commit 83e85ee into radareorg:master Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants