[WFCORE-3187] add-user.sh tool prints unexpected characters if arrow … #3064
Conversation
| @@ -24,10 +24,14 @@ | |||
|
|
|||
| package org.jboss.as.domain.management.security.adduser; | |||
|
|
|||
| import org.aesh.readline.Prompt; | |||
There was a problem hiding this comment.
These imports are in the wrong location, they should be after the static imports but before the java.* imports.
| * Class to read line from terminal using Aesh-readline library. | ||
| * Created by Marek Marusic <mmarusic@redhat.com> on 8/7/17. | ||
| */ | ||
| public class ReadLineHandler implements Consumer<Connection> { |
There was a problem hiding this comment.
Is this only used by classes in this package? If so please reduce the visibility.
| import java.io.PipedOutputStream; | ||
| import java.nio.charset.Charset; | ||
|
|
||
| import static org.junit.Assert.*; |
There was a problem hiding this comment.
The static imports need to be the first set of imports,
| @@ -34,6 +34,7 @@ | |||
| </resources> | |||
|
|
|||
| <dependencies> | |||
| <module name="org.aesh"/> | |||
There was a problem hiding this comment.
Was the org.aesh import really needed in the end on this one? This module is predominantly a dependency on org.jboss.as.domain-management with a main class specified.
There was a problem hiding this comment.
Yes, You are right. I am sorry I didn't noticed this. I fixed it along with the other suggestions from your comments. Thank You.
| @@ -71,6 +71,10 @@ | |||
| </dependencyManagement> | |||
|
|
|||
| <dependencies> | |||
| <dependency> | |||
| <groupId>org.aesh</groupId> | |||
There was a problem hiding this comment.
I will let @bstansberry also comment on this dependency.
IMO I am not overly concerned as the add-user utility is destined to be deprecated and either re-written or replaced hopefully soon.
|
I don't want aesh as a dependency of the server runtime domain-management module or the domain-management maven module. If it has to be used we need to split this tool into a separate module. Really that should be done anyway; people wanting a slim server shouldn't have to carry a client tool. We need to narrow our server dependency graph, not expand it. |
|
Ok so this is adding a dependency on aesh which the CLI already has a dependency on, I wonder if we should talk to @jfdenise about co locating with the CLI. For add-user 2.0 we have some other options to look at anyway but I would like to see if we can have a closer relationship with the CLI as we have some options to tie into management opts as well (if we want to take those options) Or another option is we move it over to wildfly-elytron-tool if we really do want a stand alone utility which also has it's own merits. |
|
@darranl , in WF12 we are introducing a new 'security' group of commands in CLI. User management would make full sense there. CLI offers some nice features (prompting, connections, ...). |
…keys are pushed during interactive mode
MMarus
force-pushed
the
WFCORE-3187-add-user.sh-prints-unexpected-characters
branch
from
f7b7953
to
dd16837
Compare
|
this is ok to test |
|
@bstansberry I think we can close this one out, within the next couple of WildFly releases we expect major changes this area likely involving collaboration with different tools. |
…keys are pushed during interactive mode
Moved #2715 to a new rebased PR for easier review.
JBEAP: https://issues.jboss.org/browse/JBEAP-6640
WFCORE: https://issues.jboss.org/browse/WFCORE-3187