-
Notifications
You must be signed in to change notification settings - Fork 175
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
Fix extra comma error in list of projects #111
Conversation
@olivergondza, please, review :) |
Looks, good to me (https://github.com/jenkinsci/parameterized-trigger-plugin/pull/111/files?w=1) but is missing tests. |
@kwhetstone, can you merge, please? |
return FormValidation.error(Messages.BuildTrigger_NotBuildable(projectName)); | ||
} | ||
if (StringUtils.isBlank(projectName)) { | ||
return FormValidation.error(Messages.BuildTrigger_NoSuchProject(" ", "?")); |
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 would rather return another warning ("Blank job name in the list")
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.
All other messages are using hudson.tasks.Messages
from core
. And most of them are localized. Is it ok to put custom message? It could break user's expectations.
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.
It's a new message, so IMHO it's fine. Jenkins core just skips such empty lines without any message:
public static <T extends Item> List<T> fromNameList(ItemGroup context, @Nonnull String list, @Nonnull Class<T> type) {
final Jenkins jenkins = Jenkins.getInstance();
List<T> r = new ArrayList<T>();
if (jenkins == null) {
return r;
}
StringTokenizer tokens = new StringTokenizer(list,",");
while(tokens.hasMoreTokens()) {
String fullName = tokens.nextToken().trim();
if (StringUtils.isNotEmpty(fullName)) {
T item = jenkins.getItem(fullName, context, type);
if(item!=null)
r.add(item);
}
}
return r;
}
Right now user can provide list of project with extra comma. I.e.
"projectA, "
And verification passes.
But after execution user receives:
It happens because in
TriggerBuilder.java
we have check like this:And as result number of tokens are bigger than number of projects (because we skipped empty one before). So, I adjust this check with the most earliest existed check I found.