fix issue #2080 #2552
fix issue #2080 #2552
Conversation
|
the ci still failed. |
|
You can run the UT before submit to upstream @cszxyang |
|
Good! @harawata I think we can review the details. |
|
@awxiaoxian2020 @Oliverwqcwrw <select id="selectFullStudent">
select
STUDENT.ID ID,
STUDENT.SNO SNO,
STUDENT.SNAME SNAME,
STUDENT.SSEX SSEX,
STUDENT.SBIRTHDAY SBIRTHDAY,
STUDENT.CLASS CLASS,
SCORE.CNO CNO,
SCORE.DEGREE DEGREE,
HOUSE.HOUSE_ID HOUSE_ID,
HOUSE.HOUSE_HOLDER HOUSE_HOLDER,
HOUSE.HOUSE_MEMBER HOUSE_MEMBER
from STUDENT, SCORE, HOUSE
<where>
<if test="sex != null">
STUDENT.SSEX = #{sex};
</if>
and STUDENT.SNO = SCORE.SNO and HOUSE.HOUSE_MEMBER = STUDENT.SNO
</where>
</select>On the other hand, to make the code of <user>
<id>100</id>
<name>Tom</name>
<age>30</age>
<cars>
<car index=\"1\">BMW</car>
<car index=\"2\">Audi</car>
<car index=\"3\">Benz</car>
</cars>
</user>Now it is like this, please let me know what you guys think of this... <user>
<id>
100
</id>
<name>
Tom
</name>
<age>
30
</age>
<cars>
<car index="1">
BMW
</car>
<car index="2">
Audi
</car>
<car index="3">
Benz
</car>
</cars>
</user> |
|
@cszxyang |
|
|
||
| assertEquals(usersNodeToStringExpect, usersNodeToString); | ||
| assertEquals(userNodeToStringExpect, userNodeToString); | ||
| assertEquals(carsNodeToStringExpect, carsNodeToString); | ||
| } | ||
|
|
||
| @Test | ||
| public void testIssue2080() { |
There was a problem hiding this comment.
IMO,The method can add comment if you want to indicate the method fix which issue,So, To modify another method name?
I agree with it and if we want to change the format, It should be another PR.IMO. |
|
@Oliverwqcwrw @awxiaoxian2020 |
Could you make a new PR so that we can test it. :) |
|
|
I recommend removing line breaks and indentation, which affects not only the readability of the code, but also the complexity of the unit tests |
|
Thanks for the comments!
Oh, I didn't realize that. Thanks for pointing that out. Just to summarize, we seem to agree that...
I'm not sure if we should remove line breaks etc. in node bodies, but we can discuss this after fixing #2080 . |
|
I think it is difficult to fix this problem without changing the original format, because you have to consider line breaks and indentation while removing the |
|
It seems we have different opinions here, I disagree with removing line breaks and indents out of user-friendliness, and I simply polished the code I submitted. |
|
It seems that the pull request has been open for a long time. The bug #2080 still existed. |
- Keeping indentation, but it won't be perfect in multi-line text nodes. - Add a new line after each open/close tag even for a short line like `<id>3</id>`. There aren't many short lines like that in MyBatis mappers. - Use 2 spaces for indentation instead of 4 to save earth. Should fix mybatis#2080 Should close mybatis#2084 mybatis#2552 mybatis#2998
- Keeping indentation, but it won't be perfect in multi-line text nodes. - Add a new line after each open/close tag even for a short line like `<id>3</id>`. There aren't many short lines like that in MyBatis mappers. - Use 2 spaces for indentation instead of 4 to save earth. Should fix mybatis#2080 Should close mybatis#2084 mybatis#2552 mybatis#2998
|
Closing in favor of #3001 |
fix this issue,. I try to get all children of the current node in the toString() method and do special handling for the
<text>node.