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

Setting list settings via C API #1944

Closed
tuespetre opened this Issue Sep 7, 2014 · 14 comments

Comments

Projects
None yet
2 participants
@tuespetre

tuespetre commented Sep 7, 2014

I've mentally stepped through the source code in reflect.h and reflect.cc to see how I should use it but I'm having some issues. If I call wkhtmltopdf_set_object_setting and try to set "load.cookies.append", I get a response of 1, indicating success. That should mean an element was added. So then, I call the method and try to set "load.cookies[0]" to a comma separated value like "my_cookie_name,mycookievalue" but I get a response of 0, indicating failure.

It seems from the source that the QPair<QString, QString> reflection implementation might be faulty, in that it seemingly returns false unless the length of the splitted value is 0, when it should check that the length is greater than 1 or maybe equal to 2 to support the subsequent lines of code.

Let me know if I'm doing something wrong. I might also get brave and try to contribute ;)

@tuespetre

This comment has been minimized.

tuespetre commented Sep 7, 2014

Oh, and also, the string checked for is "append" but the method called on the list is "push_front", which is actually a prepend, so the index accessed would always have to be 0 to get the last added element, right?

@tuespetre

This comment has been minimized.

@ashkulz ashkulz added the Verified label Sep 9, 2014

@ashkulz ashkulz added this to the 0.12.2 milestone Sep 9, 2014

@ashkulz

This comment has been minimized.

Member

ashkulz commented Sep 9, 2014

Thanks for the detailed bug reports! I'll look into this when I can.

@tuespetre

This comment has been minimized.

tuespetre commented Sep 10, 2014

OT:
The other night I was trying to build so I can start trying to contribute to some things (I'm on 64-bit Windows 8) and everything was going well until a little over halfway through compiling the Webkit dependencies. Nmake dipped out on me and complained that "there was no such file" or similar in regards to a couple of files in this folder:

https://github.com/wkhtmltopdf/qt/tree/wk_4.8.6/src/3rdparty/webkit/Source/WebCore/platform/network

The files were "NetworkStateNotifierPrivate.h" and "SocketHandlerPrivate.h". They don't show up in the link I gave you but I do see them locally. Would you have any words of wisdom for me?

@ashkulz

This comment has been minimized.

Member

ashkulz commented Sep 10, 2014

Were you using VS 2013? I've personally used both VS 2013 Express and Windows SDK 7.1 for building releases, haven't tried any other combinations. Also, check if you can compile the source tarball for 0.12.1 -- it is just a 40MiB download.

ashkulz added a commit that referenced this issue Jan 3, 2015

fix handling of QPair<QString, QString> in the reflection subsystem
The earlier code used comma as a separator and backslash for escaping,
which was neither tested or complete. Revamp it to use newline as the
separator, which should not need escaping as it is unlikely to be
present in the passed values. See #1944

@ashkulz ashkulz closed this in cd80b56 Jan 3, 2015

@ashkulz ashkulz added Fixed and removed Verified labels Jan 3, 2015

@ashkulz

This comment has been minimized.

Member

ashkulz commented Jan 3, 2015

Please test the changes and let me know if you encounter an issue, as I'm planning to make the 0.12.2 release in the next week.

@tuespetre

This comment has been minimized.

tuespetre commented Jan 3, 2015

Cool beans; will you have precompiled copies of 0.12.2-rc up on the website today? I see you bumped the version today.

@ashkulz

This comment has been minimized.

Member

ashkulz commented Jan 3, 2015

It may take a day or so, as I will be making some more changes. Note that deployment might be more complicated for you as the CRT is not linked statically (but is handled in installer).

@ashkulz

This comment has been minimized.

Member

ashkulz commented Jan 4, 2015

A release candidate 0.12.2-rc-71e97c1 is available, which should fix this issue. Please report back if your issue is not solved with the above build.

@tuespetre

This comment has been minimized.

tuespetre commented Jan 8, 2015

Reporting back to say... thanks for this one too, it's all working well

ashkulz added a commit that referenced this issue Jan 8, 2015

fix handling of QList<ComplexClass> in the reflection subsytem
This was fixed in cd80b56 but was
not complete when the template parameter was a complex class such
as PostItem, where it failed due to a leading period. Re-fixes #1944
@ashkulz

This comment has been minimized.

Member

ashkulz commented Jan 8, 2015

Please make a build yourself and see that it works, as I will release 0.12.2 final in a day or two unless more breaking changes are made.

@tuespetre

This comment has been minimized.

tuespetre commented Jan 8, 2015

I see that you've done a lot with the build configuration/process since I last tried to do my own build. I'll see what I can pull together this evening.

@ashkulz

This comment has been minimized.

Member

ashkulz commented Jan 9, 2015

No need, 0.12.2 final will be released in 12 hours or so (builds are happening right now).

@ashkulz

This comment has been minimized.

Member

ashkulz commented Jan 10, 2015

0.12.2 has been released, which includes changes related to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment