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

#953: Extend macro list #2232

Merged
merged 1 commit into from
Aug 29, 2016
Merged

#953: Extend macro list #2232

merged 1 commit into from
Aug 29, 2016

Conversation

vzhukovs
Copy link
Contributor

This PR extends existed list of macros, by adding new ones.

Related issue: #953

Created new macros to provide various information from basic part.

It includes:

${editor.current.file.name} -- Currently selected file in editor with context
${editor.current.file.path}  -- Absolute path to the selected file in editor with context
${editor.current.file.relpath} -- Path relative to the /projects folder in editor with context
${editor.current.project.name} -- Project name of the file currently selected in editor with context
${editor.current.project.type} -- Project type of the file currently selected in editor with context
${explorer.current.file.name} -- Currently selected file in project tree
${explorer.current.file.path}  -- Absolute path to the selected file in project tree
${explorer.current.file.relpath} -- Path relative to the /projects folder in project tree
${explorer.current.project.name} -- Project name of the file currently selected in explorer
${explorer.current.project.type} -- Project type of the file currently selected in explorer
${server.<name>} -- Returns protocol, hostname and port of an internal server
${server.<name>.protocol} -- Returns protocol of a server registered by name
${server.<name>.hostname} -- Returns hostname of a server registered by name
${server.<name>.port} -- Returns port of a server registered by name
${workspace.name} -- Returns the name of the workspace

@vparfonov @ashumilova @azatsarynnyy review it, please.

@benoitf
Copy link
Contributor

benoitf commented Aug 26, 2016

IMHO
${server.*} and ${workspace.*} should be handled server side

@vzhukovs
Copy link
Contributor Author

We can resolve this information on the client and also this is the only one way to do this at the moment.

@vzhukovs
Copy link
Contributor Author

Later, when we'll have mechanism to resolve it on server side, we'll move it. Maybe.

@benoitf
Copy link
Contributor

benoitf commented Aug 26, 2016

I don't see why it is the only one way as you're bringing new code to handle it, this code could be added on server instead of client

@ashumilova
Copy link
Contributor

+1
@benoitf for the server side command macros - will discuss how better to implement it and will create an issue.

@vparfonov
Copy link
Contributor

vparfonov commented Aug 26, 2016

I think we have separate issue on your idea @benoitf #2151

@benoitf
Copy link
Contributor

benoitf commented Aug 26, 2016

@vparfonov yes but on this issue it's more how to bring custom resolver by prompting the user
my concern is that a command definition inside a workspace is shared by all clients and then some properties that are only related at the server side (like mapping of ports, workspace info, etc) could be resolved for all clients by the server (and not having each client solving the properties on its own as at the end it may have different result)

@codenvy-ci
Copy link

@tolusha
Copy link
Contributor

tolusha commented Aug 26, 2016

@vparfonov @vzhukovskii
Is there any way to show the whole list of macros for user ?


import static org.mockito.Mockito.when;

public class AbstractExplorerMacroProviderTest {
Copy link
Member

Choose a reason for hiding this comment

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

javadoc with author is needed here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@vzhukovs
Copy link
Contributor Author

@tolusha at this moment - no. But we can do the same as for key binding has done. Dedicate window with table of registered bindings. For example.


import static org.mockito.Mockito.when;

public abstract class AbstractEditorMacroProviderTest {
Copy link
Member

Choose a reason for hiding this comment

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

javadoc with author is needed here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@azatsarynnyy
Copy link
Member

+1 but can't understand what does it mean - macro provider

@vzhukovs
Copy link
Contributor Author

Provider for macros

@tolusha
Copy link
Contributor

tolusha commented Aug 26, 2016

@vzhukovskii Could you pls create the issue, otherwise we will forget it ^)

@codenvy-ci
Copy link

* @since 4.7.0
*/
@Beta
public abstract class AbstractExplorerMacroProvider implements CommandPropertyValueProvider {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure we are rely need this abstraction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@vparfonov
Copy link
Contributor

not sure about Abstract*Provider's but other ok,

need ok from @benoitf

@codenvy-ci
Copy link

@TylerJewell
Copy link

TylerJewell commented Aug 28, 2016

@vparfonov @vzhukovskii - question for you. Is there any way we could add one more macro. It would be ${property.<name>} where <name> is the name of a property from che.properties file? This would allow for scenarios where a user could look up the value of a property that is in the standard list OR they could provide a custom property in che.properties.

For example:

# che.properties
my.custom.property=hamburgers

# In the macro:
${property.my.custom.property}

If this is something that requires server-side behavior, then I understand - let's add it into the issue tracking the movement of macros into the server.

@vzhukovs
Copy link
Contributor Author

@TylerJewell it requires server side modification. I think, it would be better to do this with the issue tracking the movement of macros into the server.

It includes:

${editor.current.file.name} -- Currently selected file in editor with context
${editor.current.file.path}  -- Absolute path to the selected file in editor with context
${editor.current.file.relpath} -- Path relative to the /projects folder in editor with context
${editor.current.project.name} -- Project name of the file currently selected in editor with context
${editor.current.project.type} -- Project type of the file currently selected in editor with context
${explorer.current.file.name} -- Currently selected file in project tree
${explorer.current.file.path}  -- Absolute path to the selected file in project tree
${explorer.current.file.relpath} -- Path relative to the /projects folder in project tree
${explorer.current.project.name} -- Project name of the file currently selected in explorer
${explorer.current.project.type} -- Project type of the file currently selected in explorer
${server.<name>} -- Returns protocol, hostname and port of an internal server
${server.<name>.protocol} -- Returns protocol of a server registered by name
${server.<name>.hostname} -- Returns hostname of a server registered by name
${server.<name>.port} -- Returns port of a server registered by name
${workspace.name} -- Returns the name of the workspace
@vzhukovs vzhukovs reopened this Aug 29, 2016
@vzhukovs vzhukovs merged commit 2f29a6b into master Aug 29, 2016
@vzhukovs vzhukovs deleted the #953 branch August 29, 2016 10:49
@JamesDrummond
Copy link
Contributor

@TylerJewell Custom properties will help bring in useful information about the host. Helpful for docker commands run in the host if it is possible.

@bmicklea bmicklea mentioned this pull request Aug 29, 2016
89 tasks
@bmicklea bmicklea added this to the 4.7.0 milestone Aug 30, 2016
@TylerJewell TylerJewell added the kind/enhancement A feature request - must adhere to the feature request template. label Aug 30, 2016
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
@m1L31s
Copy link

m1L31s commented Dec 10, 2018

Are those macros working?

${server..protocol} -- Returns protocol of a server registered by name
${server..hostname} -- Returns hostname of a server registered by name
${server..port} -- Returns port of a server registered by name

@vzhukovs
Copy link
Contributor Author

Hi @m1L31s! At this moment only ${server.<servername>} only work. Those three macros have already unsupported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.