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

Implement pango markup for -window-format #1288

Merged
merged 6 commits into from
May 29, 2021

Conversation

jsmnbom
Copy link
Contributor

@jsmnbom jsmnbom commented Mar 23, 2021

This PR implements pango markup for -window-format as requested in issue #1287 (see that issue for how to use).

As discussed in IRC, i believe I've done stuff mostly correct, but there might very well be some memory leaks and other issues, since I've never really worked with C and most of what I've done is guesswork...

Since g_markup_escape_text doesn't like null it seems
To bring it more in line with surrounding code
@jsmnbom
Copy link
Contributor Author

jsmnbom commented Mar 24, 2021

}
c->title = g_markup_escape_text ( tmp_title, -1 );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm.. is it possible to tmp_title to still be NULL here? Should it maybe be tmp_title ? tmp_title : ""?

@DaveDavenport
Copy link
Collaborator

FYI, been busy, will get back to reviewing this soon.

@jsmnbom
Copy link
Contributor Author

jsmnbom commented Apr 13, 2021

Oh yea, that's totally okay, i understand :3


cky = xcb_icccm_get_wm_class ( xcb->connection, c->window );
xcb_icccm_get_wm_class_reply_t wcr;
if ( xcb_icccm_get_wm_class_reply ( xcb->connection, cky, &wcr, NULL ) ) {
c->class = rofi_latin_to_utf8_strdup ( wcr.class_name, -1 );
c->class = g_markup_escape_text( c->class, -1 );
c->name = rofi_latin_to_utf8_strdup ( wcr.instance_name, -1 );
Copy link
Collaborator

Choose a reason for hiding this comment

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

This leaks memory. (c->class). confirmed with valgrind.

c->name = rofi_latin_to_utf8_strdup ( wcr.instance_name, -1 );
c->name = g_markup_escape_text( c->name, -1 );
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here.

@jsmnbom
Copy link
Contributor Author

jsmnbom commented May 23, 2021

Oh wow I had no idea valgrind was a thing, that could've come in handy earlier 😅.
I believe there's no more memory leaks now.

@codecov-commenter
Copy link

codecov-commenter commented May 23, 2021

Codecov Report

Merging #1288 (d7c22ab) into next (53533ac) will increase coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #1288      +/-   ##
==========================================
+ Coverage   32.94%   33.00%   +0.05%     
==========================================
  Files          42       42              
  Lines       11946    12038      +92     
==========================================
+ Hits         3936     3973      +37     
- Misses       8010     8065      +55     
Impacted Files Coverage Δ
source/dialogs/window.c 0.00% <0.00%> (ø)
source/dialogs/help-keys.c 83.87% <0.00%> (-8.99%) ⬇️
source/xcb.c 0.00% <0.00%> (ø)
source/rofi.c 0.00% <0.00%> (ø)
source/view.c 0.00% <0.00%> (ø)
source/dialogs/run.c 0.00% <0.00%> (ø)
source/dialogs/ssh.c 0.00% <0.00%> (ø)
source/dialogs/drun.c 0.00% <0.00%> (ø)
source/widgets/listview.c 0.00% <0.00%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53533ac...d7c22ab. Read the comment docs.

@DaveDavenport DaveDavenport merged commit 24dde58 into davatorium:next May 29, 2021
@DaveDavenport
Copy link
Collaborator

thanks.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants