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

Error reporting for unknown xml attributes #237

Open
GreenLightning opened this issue Jun 19, 2014 · 10 comments
Open

Error reporting for unknown xml attributes #237

GreenLightning opened this issue Jun 19, 2014 · 10 comments

Comments

@GreenLightning
Copy link

When there are spelling mistakes in the xml files for the gui, it is still accepted as valid by nifty, but it might corrupt the complete file. I would rather have it throw an exception while loading the file, like the runtime exception that occurs when an element with children misses the childLayout attribute.

Example:

I had a spelling mistake in the attribute. The xml was loaded fine, but afterwards nifty couldn't find any screens. This was rather hard to fix, because I only found out that something was wrong when I tried to access one of the screens (and not while loading the xml file), and the only error message I got was that the screen couldn't be found.

@void256
Copy link
Member

void256 commented Jun 19, 2014

Yes, that's a very reasonable suggestion.

However, there is a problem: Besides the attributes that Nifty supports directly (defined in the XSD) it also supports generic attributes that are not part of the XML schema. A use case for that are control parameters that are definied by the individual control and from the perspective of the XML its just a bunch of additional attributes:

<control id="checkbox" name="checkbox" checked="false" />

"checked" is not part of the allowed attributes of the "control" element!

I'm not sure how to validate the XML without losing this ability. One possibility would be to introduce parameters as nested tags, something like:

<control id="checkbox" name="checkbox">
  <param name="checked" value="false" />
</control>

That might help, I think but it would break a lot of existing XML as well 😄

Maybe something I would consider for Nifty 2.0?

@GreenLightning
Copy link
Author

Nested tags would clutter up the xml even more and as you said, they would break existing xml. I think they aren't a good option.

I don't know how nifty handles the xml, but considering that generic attributes should be included I would do it like this:

  • Separate known and unknown attributes.
  • Construct the element based on the known attributes.
  • Give the element the unknown attributes as generic attributes.

Then in the element code, do whatever is needed for the first generic attribute or throw an exception if it is unknown. Do this for all generic attributes.

(Side note: Have you considered JSON for Nifty 2.0? I'm not saying xml must die, but personally I think that JSON requires a little less typing and it's syntax is simpler / cleaner, so it might be worth thinking about...)

@void256
Copy link
Member

void256 commented Jun 19, 2014

How would that help if you misspelled a standard attribute, like f.i. you typed "heigh" instead of "height"? How to detect that? Nifty would just interpret "heigh" as a generic attribute ... that wouldn't help your original problem, right?

@relu91
Copy link
Collaborator

relu91 commented Jun 21, 2014

I really like your suggestion @void256 , it also helps nifty-editor . I talked about a problem related with this in my post at JME's forum . There are also some other possible solution to deal with custom attributes.

@GreenLightning
Copy link
Author

If "heigh" is not a valid generic attribute then whatever handles the generic attributes should throw an exception that there isn't any attribute named "heigh".

@void256
Copy link
Member

void256 commented Jun 22, 2014

Yes but I'm still not sure how to do that with the current way XML is loaded/handled in Nifty 1.x.

There are several things to consider:

  • Standard attributes would need to be marked/known as such and that depends on the specific element too (<text> has other standard attributes as <image>). Yes, I know that's part of the XSD and therefore that information is available in a way but right now, it's not available where all of that is handled.
  • What attributes are considered control parameters can be derived from the control definition. Actually that's already done somewhere inside of Nifty (parameters are definied as something="$stuff" so $stuff would be the attribute you later use: stuff="42")

So, I think, it's not something that is impossible to do just not easy with the current code base, unfortunately.

Unless someone voluntees to take a look at this I would consider this as something to do better in 2.0 instead of trying to bring that into 1.4.

It's not that I don't want to 😉 I just don't see a managable/sane solution right now ...

@GreenLightning
Copy link
Author

As I said, I don't know how nifty handles the xml.

I will probably look into this myself, but I won't have time in the next two weeks or so...

Until then I'm very happy if you just keep this as a possible feature of nifty 2.0.

@relu91
Copy link
Collaborator

relu91 commented Jun 23, 2014

I'm ok with that .

What attributes are considered control parameters can be derived from the control definition. Actually that's already done somewhere inside of Nifty (parameters are definied as something="$stuff" so $stuff would be the attribute you later use: stuff="42")

I want just you to notice that this is not true for all the controls like f.i. checkbox doesn't have any $visibile param in its control definition.

@void256
Copy link
Member

void256 commented Jun 23, 2014

Yes @relu91 ! That's another issue! Not all control attributes are part of the control definition. Sometimes only the control implementation "knows" about the attributes it supports, like in the CheckBoxControl the "checked" parameter:

  public void bind(
      final Nifty niftyParam,
      final Screen screenParam,
      final Element elementParam,
      final Properties propertiesParam,
      final Attributes controlDefinitionAttributes) {
...
    checkBoxImpl.setChecked(new Boolean(propertiesParam.getProperty("checked", "false")));
  }

@relu91
Copy link
Collaborator

relu91 commented Jul 2, 2014

Hi!, I don't know if you guys know Polymer project , but I've just found it and it looks really great! . Also I think that nifty-gui could take some cues from it. For example look here. They use an attribute to define which parameter the components will have or you could define them from Javascript.I saw a sort of parallelism between components in Polymer and Controls in nifty-gui so I think that if ever this issue will be solved ( in 1.4 or 2.0) we should keep in mind the Polymer implementation because it's a well know framework and its design is solid. What do you think??

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

No branches or pull requests

4 participants