[jira] [Comment Edited] (JCRVLT-271) Support a CLI command to format vault xml files
[ https://issues.apache.org/jira/browse/JCRVLT-271?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16371142#comment-16371142 ] Dirk Rudolph edited comment on JCRVLT-271 at 2/21/18 11:17 AM: --- I made some of the changes you recommended. I don't see a noticeable impact of sorting the attributes twice as best-case complexity of sorting is O(n) in terms of attribute count, same as other repeating parts of the logic in startElement() for example. Anyway Removing the sorting from DocViewSAXFormatter doesn't break anything in the unit tests and is not exported so it's highly unlikely that there is any consumer implicitly relying on the sorting being applied there. In terms of unit testing the current test verifies that checkFormat() returns false for a malformed file, that format() applies the right format without running into an exception asserted with checkFormat() expected to return true for the now well formatted file. I think the actual format is already covered by other tests but if you prefer I can also compare the result after formatting with a the same but well formatted file. Wdyt? was (Author: diru): I made some of the changes you recommended. I don't see a noticeable impact of sorting the attribute twice as best-case complexity of sorting is O\(n) in terms of attribute count, same as other repeating parts of the logic in startElement() for example. Anyway Removing the sorting from DocViewSAXFormatter doesn't break anything in the unit tests and is not exported so it's highly unlikely that there is any consumer implicitly relying on the sorting being applied there. In terms of unit testing the current test verifies that checkFormat() returns false for a malformed file, that format() change apply the right format without running into an exception asserted with checkFormat() expected to return true for the now well formatted file. I think the actual format is already covered by other tests but if you prefer I can also compare the result after formatting with a the same but well formatted file. Wdyt? > Support a CLI command to format vault xml files > --- > > Key: JCRVLT-271 > URL: https://issues.apache.org/jira/browse/JCRVLT-271 > Project: Jackrabbit FileVault > Issue Type: New Feature > Components: Misc >Reporter: Dirk Rudolph >Priority: Major > > In our projects we work with vlt IDE integrations (Intellij and Eclpise) to > have an easy and feature rich development process. On the other hand there > are situations where we are writing vlt xml files manually. To not have huge > diffs of formatting changes we tend to commit those vlt xml files formatted > in the way as they are produced by exporting the corresponding nodes from a > remote repository. > Unfortunately the format can not fully be achieved by formatting using the > build in xml formatters of the IDE, nor am I aware of any tooling that would > allow us to check the formatting using lets say a commit hook or maven > plugin. So the common approach we use at the moment is to push and afterwards > pull nodes to or from the remote repository. > To improve that, only formatting the local files with the format the export > uses is much more efficient and can also be automated. > This is a proposal to introduce a _format_ command for the vlt-cli that: > * accepts a list of file patterns that should be processed in the current > directory tree > * formats each file included by the patterns or > * checks if the file is in the right format and fails if not with a list of > all malformed files > This can then be used to: > * automatically format by cli invocation > * validate the format during build or as pre commit hook -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (JCRVLT-271) Support a CLI command to format vault xml files
[ https://issues.apache.org/jira/browse/JCRVLT-271?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16371142#comment-16371142 ] Dirk Rudolph edited comment on JCRVLT-271 at 2/21/18 9:33 AM: -- I made some of the changes you recommended. I don't see a noticeable impact of sorting the attribute twice as best-case complexity of sorting is O\(n) in terms of attribute count, same as other repeating parts of the logic in startElement() for example. Anyway Removing the sorting from DocViewSAXFormatter doesn't break anything in the unit tests and is not exported so it's highly unlikely that there is any consumer implicitly relying on the sorting being applied there. In terms of unit testing the current test verifies that checkFormat() returns false for a malformed file, that format() change apply the right format without running into an exception asserted with checkFormat() expected to return true for the now well formatted file. I think the actual format is already covered by other tests but if you prefer I can also compare the result after formatting with a the same but well formatted file. Wdyt? was (Author: diru): I made some of the changes you recommended. I don't see a noticeable impact of sorting the attribute twice as best-case complexity of sorting is O(n) in terms of attribute count, same as other repeating parts of the logic in startElement() for example. Anyway Removing the sorting from DocViewSAXFormatter doesn't break anything in the unit tests and is not exported so it's highly unlikely that there is any consumer implicitly relying on the sorting being applied there. In terms of unit testing the current test verifies that checkFormat() returns false for a malformed file, that format() change apply the right format without running into an exception asserted with checkFormat() expected to return true for the now well formatted file. I think the actual format is already covered by other tests but if you prefer I can also compare the result after formatting with a the same but well formatted file. Wdyt? > Support a CLI command to format vault xml files > --- > > Key: JCRVLT-271 > URL: https://issues.apache.org/jira/browse/JCRVLT-271 > Project: Jackrabbit FileVault > Issue Type: New Feature > Components: Misc >Reporter: Dirk Rudolph >Priority: Major > > In our projects we work with vlt IDE integrations (Intellij and Eclpise) to > have an easy and feature rich development process. On the other hand there > are situations where we are writing vlt xml files manually. To not have huge > diffs of formatting changes we tend to commit those vlt xml files formatted > in the way as they are produced by exporting the corresponding nodes from a > remote repository. > Unfortunately the format can not fully be achieved by formatting using the > build in xml formatters of the IDE, nor am I aware of any tooling that would > allow us to check the formatting using lets say a commit hook or maven > plugin. So the common approach we use at the moment is to push and afterwards > pull nodes to or from the remote repository. > To improve that, only formatting the local files with the format the export > uses is much more efficient and can also be automated. > This is a proposal to introduce a _format_ command for the vlt-cli that: > * accepts a list of file patterns that should be processed in the current > directory tree > * formats each file included by the patterns or > * checks if the file is in the right format and fails if not with a list of > all malformed files > This can then be used to: > * automatically format by cli invocation > * validate the format during build or as pre commit hook -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (JCRVLT-271) Support a CLI command to format vault xml files
[ https://issues.apache.org/jira/browse/JCRVLT-271?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16371142#comment-16371142 ] Dirk Rudolph edited comment on JCRVLT-271 at 2/21/18 9:33 AM: -- I made some of the changes you recommended. I don't see a noticeable impact of sorting the attribute twice as best-case complexity of sorting is O(n) in terms of attribute count, same as other repeating parts of the logic in startElement() for example. Anyway Removing the sorting from DocViewSAXFormatter doesn't break anything in the unit tests and is not exported so it's highly unlikely that there is any consumer implicitly relying on the sorting being applied there. In terms of unit testing the current test verifies that checkFormat() returns false for a malformed file, that format() change apply the right format without running into an exception asserted with checkFormat() expected to return true for the now well formatted file. I think the actual format is already covered by other tests but if you prefer I can also compare the result after formatting with a the same but well formatted file. Wdyt? was (Author: diru): I made some of the changes you recommended. I don't see a noticeable impact of sorting the attribute twice as best-case complexity of sorting is O(n) same as other repeating parts of the logic in startElement() for example. Anyway Removing the sorting from DocViewSAXFormatter doesn't break anything in the unit tests and is not exported so it's highly unlikely that there is any consumer implicitly relying on the sorting being applied there. In terms of unit testing the current test verifies that checkFormat() returns false for a malformed file, that format() change apply the right format without running into an exception asserted with checkFormat() expected to return true for the now well formatted file. I think the actual format is already covered by other tests but if you prefer I can also compare the result after formatting with a the same but well formatted file. Wdyt? > Support a CLI command to format vault xml files > --- > > Key: JCRVLT-271 > URL: https://issues.apache.org/jira/browse/JCRVLT-271 > Project: Jackrabbit FileVault > Issue Type: New Feature > Components: Misc >Reporter: Dirk Rudolph >Priority: Major > > In our projects we work with vlt IDE integrations (Intellij and Eclpise) to > have an easy and feature rich development process. On the other hand there > are situations where we are writing vlt xml files manually. To not have huge > diffs of formatting changes we tend to commit those vlt xml files formatted > in the way as they are produced by exporting the corresponding nodes from a > remote repository. > Unfortunately the format can not fully be achieved by formatting using the > build in xml formatters of the IDE, nor am I aware of any tooling that would > allow us to check the formatting using lets say a commit hook or maven > plugin. So the common approach we use at the moment is to push and afterwards > pull nodes to or from the remote repository. > To improve that, only formatting the local files with the format the export > uses is much more efficient and can also be automated. > This is a proposal to introduce a _format_ command for the vlt-cli that: > * accepts a list of file patterns that should be processed in the current > directory tree > * formats each file included by the patterns or > * checks if the file is in the right format and fails if not with a list of > all malformed files > This can then be used to: > * automatically format by cli invocation > * validate the format during build or as pre commit hook -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (JCRVLT-271) Support a CLI command to format vault xml files
[
https://issues.apache.org/jira/browse/JCRVLT-271?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16368293#comment-16368293
]
Dirk Rudolph edited comment on JCRVLT-271 at 2/17/18 5:28 PM:
--
Thank you for those information [~tripod]. With those I also added the
capability of sorting the attributes according to the format implied in the
DocViewSAXFormatter.
I moved the logic from the vault-cli to vault-core to not require consumers
(like IDE plugins) to depend on the vault-cli artefact. Additionally I moved
the logic of the attribute name sorting in AttributeNameComparator and used it
in the ItemNameComparator. The Comparator can now be set to the OutputFormat so
that the DocViewSerializer has exactly the same format as the methods for
formatting of the new DocViewFormat singleton. There is one thing I'm not sure
if it makes sense to be changed. The javadoc of XMLSerializer says:
{quote}This class and dependencies were copied from the sun jdk1.5 source base.
The original serializer is extended by a new OutputFormat setting that allows
to break the line for each attribute. additionally, *all attribute names are
sorted alphabetically*.
{quote}
which wasn't true, as until now there was no sorting of the attributes at all
in this class. Maybe we should rephrase or remove that.
+ CLA signed and I'm listed now as you requested.
was (Author: diru):
Thank you for those information [~tripod]. With those I also added the
capability of sorting the attributes according to the format implied in the
DocViewSAXFormatter.
I moved the logic from the vault-cli to vault-core to not require consumers
(like IDE plugins) to depend on the vault-cli artefact. Additionally I moved
the logic of the attribute name sorting in AttributeNameComparator and used it
in the ItemNameComparator. The Comparator can now be set to the OutputFormat so
that the DocViewSerializer has exactly the same format as the methods for
formatting of the new DocViewFormat singleton. There is one thing I'm not sure
if it makes sense to be changed. The javadoc of XMLSerializer says:
{quote}
This class and dependencies were copied from the sun jdk1.5 source base. The
original serializer is extended by a new OutputFormat setting that allows to
break the line for each attribute. additionally, *all attribute names are
sorted alphabetically*.
{quote}
which wasn't true, as until now there was no sorting of the attributes at all
in this class. Maybe we should rephrase or remove that.
> Support a CLI command to format vault xml files
> ---
>
> Key: JCRVLT-271
> URL: https://issues.apache.org/jira/browse/JCRVLT-271
> Project: Jackrabbit FileVault
> Issue Type: New Feature
> Components: Misc
>Reporter: Dirk Rudolph
>Priority: Major
>
> In our projects we work with vlt IDE integrations (Intellij and Eclpise) to
> have an easy and feature rich development process. On the other hand there
> are situations where we are writing vlt xml files manually. To not have huge
> diffs of formatting changes we tend to commit those vlt xml files formatted
> in the way as they are produced by exporting the corresponding nodes from a
> remote repository.
> Unfortunately the format can not fully be achieved by formatting using the
> build in xml formatters of the IDE, nor am I aware of any tooling that would
> allow us to check the formatting using lets say a commit hook or maven
> plugin. So the common approach we use at the moment is to push and afterwards
> pull nodes to or from the remote repository.
> To improve that, only formatting the local files with the format the export
> uses is much more efficient and can also be automated.
> This is a proposal to introduce a _format_ command for the vlt-cli that:
> * accepts a list of file patterns that should be processed in the current
> directory tree
> * formats each file included by the patterns or
> * checks if the file is in the right format and fails if not with a list of
> all malformed files
> This can then be used to:
> * automatically format by cli invocation
> * validate the format during build or as pre commit hook
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
