Support adding files in directories to context

[benthamite]

Currently, attempting to add directories to the context in Dired will throw an error. This PR provides support for adding files in directories to the context: if there is a directory at point, or the marked files include one or more directories, their files will recursively be added to the context.

[karthink]

Looks good! I'll test it when I can and merge.

[benthamite]

I just noticed this in the docstring of this function (emphasis added):

If a region is selected, add the selected region to the context. If there is already a gptel context at point, remove it instead.

I may not be understanding what this means, but as far as I can tell the function does not behave in the manner described: if I select a region, add it to the context, then re-select it and call gptel-add again, the region is not removed from the context. (The docstring talks about the "context at point" rather than the region, but I am not sure what that means in this, er, context. 😛)

[karthink]

I may not be understanding what this means, but as far as I can tell the function does not behave in the manner described: if I select a region, add it to the context, then re-select it and call gptel-add again, the region is not removed from the context.

It could probably be worded better. Here is an updated description, let me know if it makes more sense:

gptel-add is an alias for ‘gptel-context-add’ in ‘gptel-context.el’.

(gptel-add &optional ARG)

Add context to gptel in a DWIM fashion.

- If a region is selected, add the selected region to the
  context.  If point is in a context region already, it
  is removed first -- overlapping context regions are not
  supported.

- If there is already a gptel context at point, remove it
  instead.

- If in Dired, add marked files or file at point to the context.
  With negative prefix ARG, remove them from the context instead.

- Otherwise add the current buffer to the context.  With positive
  prefix ARG, prompt for a buffer name and add it to the context.

- With negative prefix ARG, remove all gptel contexts from the
  current buffer.

[karthink]

We should also add the ability to add directories to the context when running gptel-add-file, shouldn't we?

[benthamite]

We should also add the ability to add directories to the context when running
gptel-add-file, shouldn't we?

I wasn’t familiar with this command, but yeah, it seems it should mirror the behavior of gptel-add when the latter is used with a file or directory. Would you like me to make the changes?

[benthamite]

Perhaps we should change gptel-context-add-file so that this is the function called by gptel-add in Dired mode. Currently, the former doesn’t take a prefix argument.

[karthink]

Would you like me to make the changes?

Please go ahead.

Perhaps we should change gptel-context-add-file so that this is the function called by gptel-add in Dired mode

gptel-context-add-file is the function called by gptel-add in Dired mode. There is no other way to add files to the context list.

[benthamite]

@karthink, I undid the previous attempt and pushed a new version, which (1) works with both gptel-add-file and gptel-add and (2) provides the functionality to both add and remove files in directories. Let me know what you think.

[karthink]

Am I missing something here? Seems like this should be (if (gptel--file-binary-p file) (gptel-context--handle-binary file) ....

[benthamite]

Yep, not sure what happened. Will make these and the other changes shortly.

[benthamite]

Addressed: 2a40dae

[karthink]

Use pcase-exhaustive to future-proof uses.

[benthamite]

Addressed: 6c66982

[karthink]

Convert to a cond clause, it's easier to read here.

[benthamite]

Addressed: bc9138a

[karthink]

Do you think we should ask for confirmation (y-or-n-p) when adding a directory with gptel-add?

[benthamite]

I weakly lean towards asking for confirmation. The prompt could include the number of files to be added.

[benthamite]

Here’s an attempt: 8e5e3c8

Perhaps we should add a user option to allow the user to decide if they want to be asked for confirmation.

Also, note that the user will be asked once for each directory. This seems like the correct behavior, though it can become tedious if the user is adding several dirs. (Perhaps this provides an additional reason for adding the user option.)

[karthink]

This is difficult to read. How about

- If in Dired, add marked files or file at point to the context.  Directories
  will have all their files added recursively.  With negative prefix ARG, remove
  them from the context instead.

[benthamite]

Looks good.

[benthamite]

Addressed: 8bee48a