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

crete check to look for whitespace characters in xml indentation #235

Merged
merged 1 commit into from
Jan 5, 2018

Conversation

lpapazow
Copy link
Contributor

Create a check that verifies that XML files use tabs instead of spaces as suggested here

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Thanks, just a few small comments.



/**
* Cheks whether whitespace characters are use instead of tabs in xml files
Copy link
Member

Choose a reason for hiding this comment

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

typo: Checks

* Cheks whether whitespace characters are use instead of tabs in xml files
* indentations and generates warnings in such cases.
*
* @author Lyubomir Papazov - initial contribution -
Copy link
Member

Choose a reason for hiding this comment

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

what's the trailing dash for?

<module name="org.openhab.tools.analysis.checkstyle.OnlyTabIdentationInXmlFilesCheck">
<property name="severity" value="warning" />
<property name="onlyShowFirstWarning" value="true" />
</module>
Copy link
Member

Choose a reason for hiding this comment

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

This has a different indentation than the opening element.

@lpapazow lpapazow force-pushed the only-tabs-as-xml-indentation branch from 9fcd2cb to d8f8180 Compare January 5, 2018 11:53
Signed-off-by: Lyubomir V. Papazov <lpapazow@gmail.com>
@lpapazow lpapazow force-pushed the only-tabs-as-xml-indentation branch from d8f8180 to e1ca021 Compare January 5, 2018 11:56
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

That was quick, thanks!

@kaikreuzer kaikreuzer merged commit d6d7199 into openhab:master Jan 5, 2018
@kaikreuzer kaikreuzer added this to the 0.4.0 milestone Jan 15, 2018
*/
public class OnlyTabIdentationInXmlFilesCheck extends AbstractStaticCheck {

private static final String PATTERN_TO_BE_FOLLOWED = "^\\t*[<].*";
Copy link
Member

@martinvw martinvw Jan 28, 2018

Choose a reason for hiding this comment

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

This regex is just wrong. Should you maybe rather check on lines starting with spaces?

According to this regex you can not have lines with multi-line CDATA blocks, you can not split your header like the common example.

<thing:thing-descriptions bindingId="onewiregpio" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
	xmlns:thing="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0"
	xsi:schemaLocation="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0 
        http://eclipse.org/smarthome/schemas/thing-description-1.0.0.xsd">

Copy link
Member

Choose a reason for hiding this comment

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

And quickly reading its not even allowed to have empty lines?

Copy link
Contributor Author

@lpapazow lpapazow Jan 29, 2018

Choose a reason for hiding this comment

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

I didn't think of those cases. I'll change it asap.

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.

3 participants