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
Xml attribute tidy up #4971
Xml attribute tidy up #4971
Conversation
…ion of an attribute
jenkins build this please |
1 similar comment
jenkins build this please |
Couldn't find anything wrong. Nice cleanup/refactor. |
bSort=true; | ||
|
||
if (type) | ||
const std::string type = XMLUtils::GetAttribute(setting, "type"); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
…ectly to CStdString/std::string, as they may be NULL
…pared case-sensitive
… made case-sensitive
…sed enough to warrant the include)
…-insensitive (we write them, we read them)
…ase-sensitive to drop some strcmpi()
…exps case-sensitive (i.e. 'true')
Thanks for the review! Updated as per comments. jenkins build this please |
There is a problem with PR4971 - now that this is merged, is it better to open a new issue? Assuming the discussion can continue here, the addon Artwork Downloader is missing several label and textual elements from the Settings dialog once this PR is added. Before #1 - without PR4971 (fde760b): After #1 - with PR4971 (a362942): Before #2 - without PR4971 (fde760b): After #2 - with PR4971 (a362942): See also here. Given the changes made by this PR, I suppose it's equally possible that these addons that are now missing textual elements may require updating to be case-sensitive (ping @MartijnKaijser). |
Add-on settings were already case-sensitive in bits, but not case-sensitive in other bits, so yes, it needs fixing in the add-ons. |
This isn't due to case-insensitivity, however. Addon Setting controls are no longer being created in GUIDialogAddonSettings.cpp unless an The Artwork Downloader "Info" panel above that is no longer working can be viewed here. It looks correct, and should be displayed, but isn't. Removing the null check on |
Thanks for investigating - most useful. It's not quite as easy as removing the null check, as that's needed, but perhaps can be moved to where it's needed (we don't care about it for separator controls for example). Will look into it. |
Nothing needs to change in the addons, this PR needs updating so that it doesn't ignore controls with no |
OK, I understood that something could be made in the setting so the controls would appear, but that way there's nothing to do in the addon matter. Thanks, will wait for PR update. |
@Mafarricos - the sep/lsep issue is fixed by #5025 which is in build #0713 on the forum. If you find any problems please update #5025, thanks. |
@MilhouseVH it seems that all is fixed with this PR. |
This includes fix for possible crashes (null ptr deref) which should be backported to Gotham, plus additional fixes for not reading Attribute's into CStdString (which has null ptr protection) so that when we replace with std::string we don't introduce a bunch of further null ptr deref's.
It mostly touches various settings:
$HOURS
is used, that is now case-sensitive as well.bydate
attribute of AdvancedSettings tvshow matching regexp is now case-sensitive.Otherwise, just general cleanup.
This needs to go in before the next CStdString removal, so sooner the better on reviews please!