Skip to content
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

Jbide13135a40x v2 #19

Merged
merged 3 commits into from
Nov 28, 2012

Conversation

robstryker
Copy link
Member

PR for startup args from a windows machine to a RSE server on linux machine (plus test case)

@robstryker
Copy link
Member Author

There were some issues which may make it a tiny bit confusing. First, one of the constants was improperly named. It was named HOME_URL instead of BASE_URL, which is a big issue IMO, not just cosmetic. I believe that's what caused some of the confusion in the past.

That had to change. So LOCAL servers are still url based. that's why you don't see a change there. But local urls are very easy to craft via java.io.File().

The RSE portion, though, is now dir based instead of url based. It also does its absolute best to determine if the remote system is windows or not. If it was created via the windows system type, then it'll accurately detect. If it has any windows subsystem (like dstore.windows.something), it'll correctly identify.

If however the user creates an ssh-only system to a remote windows machine running open-ssh, it will mis-identify. But as of now, that's the best I can do.

@@ -104,9 +105,19 @@ private String getDefaultLaunchCommand(ILaunchConfiguration config) throws CoreE
String currentArgs = config.getAttribute(IJavaLaunchConfigurationConstants.ATTR_PROGRAM_ARGUMENTS, ""); //$NON-NLS-1$
String currentVMArgs = config.getAttribute(IJavaLaunchConfigurationConstants.ATTR_VM_ARGUMENTS, ""); //$NON-NLS-1$

// Clear old flag which used url
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean users can never set jboss.server.home.url explicitly ? (since it gets forcefully nulled out?)
Probably not a big deal, but just wondering.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. Since we forcefully override the .dir flag, users are forbidden from using the .url flag. However they can still customize the command, at which point we do not update anything and give them full control over the remote command

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha. thats fair.

* 'jboss.server.home.url'
* Any consumers who may be affected should change now
*/
@Deprecated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robstryker robstryker merged commit 6b89d49 into jbosstools:jbosstools-4.0.x Nov 28, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants