Fix #422: Markdown formatting for chat history

[jeanlucthumm]

[jeanlucthumm]

@TheR1D is there any procedure I need to do for assigning a reviewer?

[jeanlucthumm]

Ah ok good catch. Before enabling it only for default, what are your thoughts on wrapping assistant messages with markdown code block if we are in code mode? I think that would look very nice and it makes sense because the assistant is outputting code after all.

We can do this only for show_messages() by detecting mode and whether it's an assistant message.

The main concern here would be not outputting markdown wrapper when user is expected to redirect to file. But I think the use case for that is only this:

sgpt --code --chat c1 "a python file that says hello" > hello.py
sgpt --code "a python file that says hello" > hello.py

Both of which do not use show_messages

[TheR1D]

Ah ok good catch. Before enabling it only for default, what are your thoughts on wrapping assistant messages with markdown code block if we are in code mode? I think that would look very nice and it makes sense because the assistant is outputting code after all.

We can do this only for show_messages() by detecting mode and whether it's an assistant message.

The main concern here would be not outputting markdown wrapper when user is expected to redirect to file. But I think the use case for that is only this:

sgpt --code --chat c1 "a python file that says hello" > hello.py
sgpt --code "a python file that says hello" > hello.py

Both of which do not use show_messages

You're right, we can use markdown code block when showing --code conversations with show_messages. May be we can apply md formatting for --shell conversations as well. For instance wrap assistant responses with "inline code" using backticks `, e.g.ls -la | sort`

[jeanlucthumm]

Sounds good. I'll need to change how conversations are stored for this because we don't currently store the role along side the messages, only a json list, so show-chat has no way of knowing.

My idea is to change the format from a list of messages to an object like this:

{
  "role": "...",
  "messages": [
    ...
  ],
}

This is also good for future if we ever want to store more metadata about the conversations.

Will take a look this weekend.

[jeanlucthumm]

Ok, @TheR1D please take a look.

Some demos:

[TheR1D]

Thank you, @jeanlucthumm. I will review it during this week. I believe we may need to make some changes.

[TheR1D]

Hey @jeanlucthumm, thank you for the PR. After reviewing the changes, I believe this feature could be implemented more concisely. We can reuse the initial_message method available in ChatHandler. Here's an example:

@classmethod
def initial_message(cls, chat_id: str) -> str:
    chat_history = cls.chat_session.get_messages(chat_id)
    return chat_history[0] if chat_history else ""

Then we can slightly change logic in show_messages.

@classmethod
def show_messages(cls, chat_id: str) -> None:
    # Prints all messages from a specified chat ID to the console.
    init_message = cls.initial_message(chat_id)

    if "use and apply markdown" in init_message.lower():
        for message in cls.chat_session.get_messages(chat_id):
            if message.startswith("assistant"):
                Console().print(Markdown(message))
            else:
                typer.secho(message, fg=cfg.get("DEFAULT_COLOR"))
            typer.echo()
        return

    for index, message in enumerate(messages):
        color = "magenta" if index % 2 == 0 else "green"
        typer.secho(message, fg=color)

I think this is more suitable solution since we are following the system prompt, and it would be compatible with custom roles. It will not wrap --code conversations into a code block, but after giving it some more thought, I don't see many benefits in rendering the code using a markdown block. I think --code will mostly be used when a user wants to redirect clean code output elsewhere. If Markdown is required, we can prompt code generation using the default ShellGPT role. Could you also include tests to make sure it works as expected?

Thank you for interest and effort.

[jeanlucthumm]

Sure, let's do it this way. Will take a look this weekend!

[jeanlucthumm]

@TheR1D Ok I implemented your suggested change and also added unit tests. I also updated the documentation for custom roles so users are aware of this functionality.

Also let me know if you'd like me to squash commits or anything like that.

[jeanlucthumm]

Ok finished!

[TheR1D]

Thank you @jeanlucthumm, merged.