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

Use new jsoncpp reader #4

Merged
merged 1 commit into from
Dec 3, 2017
Merged

Conversation

patrick96
Copy link
Member

@patrick96 patrick96 commented Oct 18, 2017

Json::Reader is deprecated and will fail polybar compilation

EDIT: I still have to test if the new function call still works the same

Fixes polybar/polybar#741 and polybar/polybar#871

Json::Reader is deprecated and will fail polybar compilation

Fixes polybar/polybar#741
@patrick96
Copy link
Member Author

I have tested this a little but since I do not use i3 as my regular workspace, the test was in no way comprehensive. But I think the jsoncpp people made sure the CharReader behaves the same as the now deprecated Reader
Also other people have already reported that this patch works and doesn't cause any problems, so I'm going to merge it.

@patrick96 patrick96 changed the title [WIP] Use new jsoncpp reader Use new jsoncpp reader Dec 3, 2017
@patrick96 patrick96 merged commit a6aa7a1 into polybar:master Dec 3, 2017
patrick96 added a commit to patrick96/polybar that referenced this pull request Dec 3, 2017
Actual fix was made in polybar/i3ipcpp#4 this only
updates the submodule

Fixes polybar#741
Fixes polybar#871
@patrick96 patrick96 deleted the reader_deprecated branch December 3, 2017 16:11
@patrick96
Copy link
Member Author

Totally forgot about testing this before merging, but this also works with jsoncpp 1.7.7 that ships with i3ipcpp, so it should also work for all versions in between (hopefully)

patrick96 added a commit to polybar/polybar that referenced this pull request Dec 3, 2017
Actual fix was made in polybar/i3ipcpp#4 this only
updates the submodule

Fixes #741
Fixes #871
}
{ \
Json::CharReaderBuilder b; \
Json::CharReader* reader(b.newCharReader()); \
Copy link

Choose a reason for hiding this comment

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

Doesn't this leak memory?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, probably does. I'm gonna open a PR

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.

2 participants