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
Add needle management #2132
Add needle management #2132
Conversation
It's a quick first draft
@@ -8,6 +8,7 @@ | |||
|
|||
abstract class JHipsterModuleReplacements { | |||
|
|||
public static final String LF = "\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps use System.getProperty("line.separator")
? or get from project ? instead of "\n"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should "force" \n as line break everywhere, using something else is always a pain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok but shouldn't that be set as a project property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a mistake to use a property for that since anything else that \n will be a nightmare for the project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, there were several discussion about that at the beginning of this project. If we can force the use of \n
for Windows too, it would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can make an issue to make this on the module part so this will naturally make it's way :)
Codecov Report
@@ Coverage Diff @@
## main #2132 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 2407 2425 +18
===========================================
Files 517 521 +4
Lines 8886 8929 +43
Branches 201 203 +2
===========================================
+ Hits 8886 8929 +43
Continue to review full report at Codecov.
|
Assert.notNull("element", element); | ||
} | ||
public String updateReplacement(String value) { | ||
return element.searchMatcher().concat(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
element -> element()
public JustAfter { | ||
Assert.notNull("element", element); | ||
} | ||
public String updateReplacement(String value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing line break before method
public JustBefore { | ||
Assert.notNull("element", element); | ||
} | ||
public String updateReplacement(String value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing line break before method
Assert.notNull("element", element); | ||
} | ||
public String updateReplacement(String value) { | ||
return value.concat(element.searchMatcher()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
element -> element()
Assert.notNull("element", element); | ||
} | ||
public String updateReplacement(String value) { | ||
return element.searchMatcher().concat(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert.notBlank(value)
@@ -8,6 +8,7 @@ | |||
|
|||
abstract class JHipsterModuleReplacements { | |||
|
|||
public static final String LF = "\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should "force" \n as line break everywhere, using something else is always a pain
public JustAfter { | ||
Assert.notNull("element", element); | ||
} | ||
public String updateReplacement(String value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Override
public JustBefore { | ||
Assert.notNull("element", element); | ||
} | ||
public String updateReplacement(String value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Override
@@ -0,0 +1,7 @@ | |||
package tech.jhipster.lite.generator.module.domain.replacement; | |||
|
|||
interface PositionalMatcher { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a needle matcher?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not specifically needle, but all matcher.
src/main/java/tech/jhipster/lite/generator/module/domain/replacement/PositionalMatcher.java
Outdated
Show resolved
Hide resolved
There are some conversation left open but it's OK for me, @antarus still want to work on this? |
I Can work on it but i think I made all the requests |
Close #2104
It's a quick first draft