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

IDE Integration #247

Open
simonharrer opened this issue Mar 27, 2022 · 19 comments
Open

IDE Integration #247

simonharrer opened this issue Mar 27, 2022 · 19 comments

Comments

@simonharrer
Copy link
Member

Why

There is the really great intellij mob plugin https://github.com/remotemobprogramming/intellij-mob. It is really well integrated with IntelliJ and works great. It is a port of the mob cli tool. And both tools are no longer in sync. They might even already be incompatible.

A few days back, a plugin for VS code has been developed https://github.com/remotemobprogramming/mob-vscode-gui . This uses the mob cli tool to be in sync with the CLI tool - it just is a wrapper around, although a great one, around the cli tool. But has a hard time as it has to parse the stdout to determine the outcome of any executed mob commands.

Idea

The CLI tool should provide responses in a JSON format with some kind of IDs on which IDEs can work on. It might also provide a way to pass in configuration options are JSON instead of having to set environment variables for each CLI call.

@hollesse
Copy link
Member

What information should be in the json response? Are you thinking about error codes?

@alessandrosangalli
Copy link
Contributor

Just a field saying that an error occourred and other with the stdout helps a lot, maybe a good first step before error codes.

@simonharrer
Copy link
Member Author

@alessandrosangalli for which commands do you need that? Or just in general? Also curious, which commands do you use in your plugin? Do you use any of the more info-related commands?

@gregorriegler
Copy link
Collaborator

So the Idea is to create a mode in which mob.sh speaks JSON - it reads JSON and prints JSON?
Then we are in Content-Negotiation territory.
For each required Input and Output we should then have data-transfer-structs to abstract away the representational details (JSON / Text).
We would serialize the JSON or Text from those structs and vice versa. Similar to MV* frameworks, but as simple as possible.
It would need quite some preparatory refactoring to introduce such structure, before we can add the JSON mode.
The code would become longer more complex. In order to stay readable it would have to be split in separate modules/files IMO.
We could also try it with minimum structure change, but I assume it will become more and more like spaghetti.

@alessandrosangalli
Copy link
Contributor

@alessandrosangalli for which commands do you need that? Or just in general? Also curious, which commands do you use in your plugin? Do you use any of the more info-related commands?

Now i just use start, next, done, timer and reset. No info-related commands.
I've been using the VSCode plugin daily for the last week with my team and it's been enough.

@alessandrosangalli
Copy link
Contributor

So the Idea is to create a mode in which mob.sh speaks JSON - it reads JSON and prints JSON?
Then we are in Content-Negotiation territory.
For each required Input and Output we should then have data-transfer-structs to abstract away the representational details (JSON / Text).
We would serialize the JSON or Text from those structs and vice versa. Similar to MV* frameworks, but as simple as possible.
It would need quite some preparatory refactoring to introduce such structure, before we can add the JSON mode.
The code would become longer more complex. In order to stay readable it would have to be split in separate modules/files IMO.
We could also try it with minimum structure change, but I assume it will become more and more like spaghetti.

I believe that we dont need read json, just print when call mob with something like "--json" (@simonharrer can correct me).

I think something like {message: "current output", error: null or some error code or just "true"} should be nice to a first step, maybe in the future we need more complex things, it's just my point of view as someone who wraps mob.sh in a vscode extension.

@hollesse
Copy link
Member

hollesse commented Apr 2, 2022

So the Idea is to create a mode in which mob.sh speaks JSON - it reads JSON and prints JSON?
Then we are in Content-Negotiation territory.
For each required Input and Output we should then have data-transfer-structs to abstract away the representational details (JSON / Text).
We would serialize the JSON or Text from those structs and vice versa. Similar to MV* frameworks, but as simple as possible.
It would need quite some preparatory refactoring to introduce such structure, before we can add the JSON mode.
The code would become longer more complex. In order to stay readable it would have to be split in separate modules/files IMO.
We could also try it with minimum structure change, but I assume it will become more and more like spaghetti.

Could it be an option to just refactor the log Functions, such that they create json objects with a message and a warning and error flag? This could be a simple solution and a first step.

@gregorriegler
Copy link
Collaborator

Could it be an option to just refactor the log Functions, such that they create json objects with a message and a warning and error flag? This could be a simple solution and a first step.

Yes, output only makes it simpler. But we still need some kind of negotiation code, for which output to generate (Json or plain text), in the code that handles the output.
So this code will grow I guess. Thinking we could put output in it's own file for example.

@simonharrer
Copy link
Member Author

Thanks for your points. I suggest to focus first on the API, and then think on how we can implement this.

Current commands: start, next, done, timer, and reset. I suggest, we ignore the timer for now, and focus on the main commands only, namely: start, next, done, and reset. We can add more commands later.

The current idea: When passing in the flag --json the commands will return a json object on stdout and no other output. I am currently really unsure, what we should put in that JSON. We could provide a lot of info, like the current branch, the mob command that caused this json response, all log messages separated by log level, etc. We could start with a status representing either success or error, and provide the logs in some other form. Can you give us a list of log outputs you currently check in your plugin @alessandrosangalli to give us a better idea on the current use case?

@alessandrosangalli
Copy link
Contributor

Thanks for your points. I suggest to focus first on the API, and then think on how we can implement this.

Current commands: start, next, done, timer, and reset. I suggest, we ignore the timer for now, and focus on the main commands only, namely: start, next, done, and reset. We can add more commands later.

The current idea: When passing in the flag --json the commands will return a json object on stdout and no other output. I am currently really unsure, what we should put in that JSON. We could provide a lot of info, like the current branch, the mob command that caused this json response, all log messages separated by log level, etc. We could start with a status representing either success or error, and provide the logs in some other form. Can you give us a list of log outputs you currently check in your plugin @alessandrosangalli to give us a better idea on the current use case?

Today, i expect each of command returns a success output:

start: "Happy collaborating!"
next: "git push --no-verify"
done: "To finish, use"
reset: ["Branches", "deleted"]
timer: "Happy collaborating!"

If the expected message isn't in the output, i pop-up the mob stdout error to the user check. Any way of explicit say's that an error occour is nice for me, because today if some message of mob changes my error handling stop work

@simonharrer
Copy link
Member Author

simonharrer commented Apr 3, 2022

Thanks for the quick response.

All those messages basically verify that the result was successful.

The mob tool basically knows two result status: success and error. In case of an error, the mob tool optionally offers a fix to overcome the current error. Both sucess and error should provide more info for the user as well. So here's my first idea on a json structure:

{
  "status": "success",
  "output": "THE FULL OUTPUT WITH NEWLINES AS A SINGLE STRING"
}
{
  "status": "error",
  "output": "THE FULL OUTPUT WITH NEWLINES AS A SINGLE STRING"
}

This shows the error result, but with a fix directly.

{
  "status": "error",
  "fix": {
      "text": "To start, including uncommitted changes, use",
      "command": "mob start --include-uncomitted-filed",
  },
  "output": "THE FULL OUTPUT WITH NEWLINES AS A SINGLE STRING"
}

How do you feel about that? Or do we an additional errorDetails field?

@hollesse
Copy link
Member

hollesse commented Apr 3, 2022

How about

{
  “status”: “error”,
  “fix”: {
    “text”: “…”,
    “command”: “…”
  },
  ”logs”: [
    {
      “text”: “…”,
      “level”: “error”
     },
     …
   ]
}

Then it would be easy to look for warnings and errors and show the messages to the user. Or do you think this doesn’t bring any benefit?

@simonharrer
Copy link
Member Author

Not really regarding the errors. Because after an error, the tool terminates. It would only help with warnings, but we don't really have a lot of them. But checking the whole output would require joining the string for user.

@hollesse
Copy link
Member

hollesse commented Apr 3, 2022

Okay. But would the output in your example just be the error message or all the logs produced by running the mob command?

@simonharrer
Copy link
Member Author

That is my question with regard to errorDetail that would contain only the error messages and output would contain everything.

@hollesse
Copy link
Member

hollesse commented Apr 3, 2022

I think error details would be good. Because this could then directly be used to show it to the user if there is no fix for an error.

@gregorriegler
Copy link
Collaborator

Would like to rename fix.text with fix.instruction
and possibly output with message

@simonharrer
Copy link
Member Author

Awesome comments. Thank you all for them!

Current draft. Feel free to comment further! next and fix are optional.

{
  "status": "success",
  "next": {
        "instruction": "To finish, use",
        "command": "git commit",
    },
  "message": "THE FULL OUTPUT WITH NEWLINES AS A SINGLE STRING"
}
{
  "status": "error",
  "fix": {
      "instruction": "To start, including uncommitted changes, use",
      "command": "mob start --include-uncomitted-filed",
  },
  "errorMessage": "ALL ERROR MESSAGE OUTPUTS",
  "message": "THE FULL OUTPUT WITH NEWLINES AS A SINGLE STRING"
}

@JThoennes
Copy link

Hello, I tried out this IntelliJ plugin but it is incompatible with the CLI mob.sh as mentioned above.
Any plans to improve this?

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

No branches or pull requests

5 participants