pomutils icon indicating copy to clipboard operation
pomutils copied to clipboard

-Xignore-all-space not respected

Open borisbrodski opened this issue 7 years ago • 15 comments

First of all, thank you for this awesome project!

We are using it for a quite a bit. It works flowerless with the exception of just one issue:

If we merge with

$ git merge -Xignore-all-space other-branch

the pom.xml files, that get processed my the pomutils merge engine get conflicts, if they contain differences in space/tab indentation.

Put it simple, the -Xignore-all-space git merge flag doesn't get respected for the files, on which pomutils was involved.

  • Are there a trick to avoid it?

I'm looking into implementing an additional switch, like --expandtab 4, that will convert all tabs into 4 spaces in both files before exiting.

  • Do you have better suggestions?
  • Are you interested in a push request?

borisbrodski avatar Dec 12 '18 08:12 borisbrodski

Hi,

i had a quick look and it is not that easy. We only adjust the pom version value element, regardless of the position on the line.

Example: branch1: <version>1.0</version> branch2: ....<version>2.0</version> (where dots are spaces)

If you do a branch 2 --> branch1 merge, the result (before the real merge happens) will be:

  • if "our" strategie is used <version>1.0</version>
  • if "their" strategie is used <version>2.0</version>

If you do a branch 1 --> branch2 merge, the result (before the real merge happens) will be:

  • if "our" strategie is used ....<version>2.0</version> (where dots are spaces)
  • if "their" strategie is used ....<version>1.0</version> (where dots are spaces)

After we adjusted the value, we call the git merge driver git merge-file ... and this does not have any options like ignore-all-space.

I need to think about it a little bit more, if there is a solution....

cecom avatar Dec 12 '18 11:12 cecom

Thank you for the quick response. I also couldn't find a simple solution to the missing --ignore-all-space switch. One complicated solution may be:

  • create a new temporary directory and git init there
  • Add base-file to a repo and commit it
  • Create branch1, add current-file and commit it
  • Create branch2 on the base commit, add other-file and commit it
  • Issue git merge branch1 -Xignore-all-space
  • Grab the merged file
  • Remove temporery directory

But if you would consider a simpler problem, like just confused tabs and spaces, the solution may be also much simpler. For example:

  • branch1: \t<version>1.0</version> (where \t is the tab)
  • branch2: ....<version>2.0</version> (where dots are spaces)

you may just come away with the simple TAB -> SPACE replacement. I already tested it by adding

  • --expandtab=<count-of-spaces>

option to the pomutils, that just replaces all TABs in

  • current
  • base
  • other

by 4 spaces (in my example). This solves my current problem. Of course, if TABs doesn't exactly match SPACES, this will fail as you have described.

borisbrodski avatar Dec 12 '18 13:12 borisbrodski

The problem on your first solution is, that would happen for all poms. So if you have a multi module project with 100 of poms that could take a while, because it would be done for every single pom.

Your Second solution does not work, because i dont have access to the real pom file. I manipulate the file via the maven-version-plugin classes and they dont provide any access to the real file.

Update: For your second solution, i only want to update the line where the version "problem" exists. But you meant to replace all \t in the complete file, correct?

cecom avatar Dec 12 '18 13:12 cecom

Yes, I replace all \t into spaces. This already works here. Do you find it wrong to replace all \t ?

And yes, the performance will be definitely the problem. On the other hand, solving conflicts manually is always longer, that any automated procedure. Also I would not use this first complex solution by default.

borisbrodski avatar Dec 12 '18 13:12 borisbrodski

If there is no solution on your side (get a xml formatter for every developer so the pom.xml file is always correct formatted), then the way i would go would be:

  • add a property ala "preFormatXml" which defaults to "false"
  • if true, before we do anyhing with the pom, we format the complete pom with the formatter, so everything is alined after a template
  • after that we do our merge stuff.

That would solve \t and space issues. But im a litte bit unsure. There could also be some formatting changes in your pom, which you dont want...

cecom avatar Dec 12 '18 14:12 cecom

Thank you!

This is working great so far. Instead of full format, I am just replacing tabs with spaces.

borisbrodski avatar Feb 07 '19 09:02 borisbrodski

so you changed your developers format rules and the problem does not exist anymore? So we can close this issue?

cecom avatar Feb 27 '19 08:02 cecom

No, I extended pomutils to simply replace all tabs with spaces before passing files to the git merge.

borisbrodski avatar Feb 27 '19 12:02 borisbrodski

Ok, than send me a pull request and i will have a look. Plz add a test too. Have a look at my other tests, its easy to add them.

cecom avatar Feb 27 '19 12:02 cecom

@borisbrodski are you working on it anymore? Or can i close this ticket?

cecom avatar Apr 23 '20 16:04 cecom

Thank you for reminding me of this issue. I will add some tests and create a pull request.

borisbrodski avatar Apr 24 '20 09:04 borisbrodski

Please, check the pull request :)

borisbrodski avatar May 15 '20 15:05 borisbrodski

Hi Boris,

i had a look. What do you think, if i would change it to "Tabify" and "Untabify"? So we can get both worlds? The one who uses TABs can use the tabify and one who uses Spaces uses the Untabify. I would take your Code and change it a little bit :-)

cecom avatar Jun 14 '20 09:06 cecom

Hi Cenom,

sure, go for the change. But please, accept pull request first and then make additional commit with your change. It allow me to merge back your change quickly (in my branch) and it will also preserve the authorship of the feature ;)

Thank you!

borisbrodski avatar Jun 14 '20 11:06 borisbrodski

For sure. I will take your branch and work on it.

cecom avatar Jun 14 '20 11:06 cecom