s-kostyaev / ellama

Ellama is a tool for interacting with large language models from Emacs.
GNU General Public License v3.0
378 stars 27 forks source link

Consider removing the fill-region call (or limit it to ellama-ask) #35

Closed Stebalien closed 7 months ago

Stebalien commented 7 months ago

It tends to mess with code formatting. Personally, I just use visual-fill-column-mode but maybe there's a middle ground?

s-kostyaev commented 7 months ago

I think we should limit fill-region usage to commands without code. I can confirm it leads to problems with code formatting, but I'd like to use it for non-code text.

Stebalien commented 7 months ago

Unfortunately, any command can produce code. Although, I guess, markdown will avoid filling code blocks once they have a closing fence.

s-kostyaev commented 7 months ago

I think we should for now make new callback like (formatcb start end) and set it to fill-region for ellama-chat

s-kostyaev commented 7 months ago

Maybe there is same function, but safe for code. Or we can try to write it ourselfs. Maybe with conditions on major mode or something like that.

s-kostyaev commented 7 months ago

We can get some ideas from auto-fill-mode - it works fine with both code and text.

Stebalien commented 7 months ago

So, there are two separate issues:

  1. Joining multiple lines of code together.
  2. Wrapping lines of code.

The first case can be solved by preserving all explicit newlines when wrapping. We can do this by:

  1. Binding use-hard-newlines while reformatting.
  2. Setting the hard text property to t for all newlines explicitly output by the LLM.

I'm not sure how to solve the second case.

s-kostyaev commented 7 months ago

So, there are two separate issues:

  1. Joining multiple lines of code together.
  2. Wrapping lines of code.

The first case can be solved by preserving all explicit newlines when wrapping. We can do this by:

  1. Binding use-hard-newlines while reformatting.
  2. Setting the hard text property to t for all newlines explicitly output by the LLM.

I'm not sure how to solve the second case.

this works:

(add-text-properties start (point) '(hard))

But fill-region can add newlines inside comments and break the code.

s-kostyaev commented 7 months ago

@Stebalien check version 0.4.8

s-kostyaev commented 7 months ago

Looks like it doesn't help.

s-kostyaev commented 7 months ago

fill-region will be called only for non-programming modes. Version 0.4.9

s-kostyaev commented 7 months ago

@Stebalien can we close this?

Stebalien commented 7 months ago

Yeah, that works reasonably well. It still messes with code blocks in markdown while in-progress, but everything is fine when the command finishes (i.e., once the markdown code block is complete).

Would you mind if I submit a patch to make filling optional (via a customization option)?

s-kostyaev commented 7 months ago

Yeah, that works reasonably well. It still messes with code blocks in markdown while in-progress, but everything is fine when the command finishes (i.e., once the markdown code block is complete).

Would you mind if I submit a patch to make filling optional (via a customization option)?

Sure. Let it enabled by default.