gptel-org: put hardcoded gptel-org-property-names into variable

[hrehfeld]

so other (potentially external) code can reference them.

e.g. I have org property inheritance disabled but want it enabled for gptel props:

  (cl-loop
   for prop in '("GPTEL_SYSTEM" "GPTEL_BACKEND" "GPTEL_MODEL"
            "GPTEL_TEMPERATURE" "GPTEL_MAX_TOKENS"
            "GPTEL_NUM_MESSAGES_TO_SEND")
   do
   (add-to-list 'org-use-property-inheritance prop))

becomes

  (cl-loop
   for prop in gptel-org-property-names
   do
   (add-to-list 'org-use-property-inheritance prop))

GPTEL_BOUNDS is purposely not included, because it seems like only used file globally.

[karthink]

@hrehfeld thanks for the PR. While I'm okay with placing these in a variable, you can achieve the effect you want more easily via

(setq org-use-property-inheritance "^GPTEL_")

Any reason you don't want to use this?

1. Could you make this defvar a defconst?
2. Do you think the name gptel-org-properties might be better? If Org mode already has a convention for one of these (-properties vs -property-names for lists of properties) we can stick to it.

As an aside, not making these properties inheritable by default is intentional, see my comment about this in #141.

[hrehfeld]

Fixed defconst. Also improved documentation, gave my use case as an example.

[hrehfeld]

@hrehfeld thanks for the PR. While I'm okay with placing these in a variable, you can achieve the effect you want more easily via

(setq org-use-property-inheritance "^GPTEL_")

Any reason you don't want to use this?

1. Could you make this `defvar` a `defconst`?

2. Do you think the name `gptel-org-properties` might be better?  If Org mode already has a convention for one of these (`-properties` vs `-property-names` for lists of properties) we can stick to it.

As an aside, not making these properties inheritable by default is intentional, see my comment about this in #141.

Well, unless you're claiming the whole GPTEL_ property namespace ;-) it just feels less robust. And I figure my PR wouldn't cause you more work, as you're listing the properties anyways.

[hrehfeld]

But wait. The code where list was hardcoded before is actually a pcase-let. So one CAN'T easily put new property names into my constant without also changing that code. (pcase-let doc: a mismatch may signal an error or may go undetected, binding variables to arbitrary values, such as nil.).

Which I did, I added GPTEL_TOPIC -- which doesn't make any sense, as TOPIC should always be inherited.

So we'd actually need another defconst just for that destructure, or hardcode it anyways and just ADD the constant. Or just give up on this?

Maybe the constant should be named something like gptel-org-property-names-selectively-inherited? 🫣

[karthink]

@hrehfeld You're right, this function combines property names with separate logic for handling each corresponding property value. So adding or removing topics from the defconst won't be enough, you'll also have to provide the logic. I'm not sure it's worth writing and managing separate functions for handling each property at this point.

Well, unless you're claiming the whole GPTEL_ property namespace

I'm not seeing the problem with this, at least as far as inheritance is concerned?

[karthink]

So we'd actually need another defconst just for that destructure, or hardcode it anyways and just ADD the constant. Or just give up on this?

I'm leaning towards closing this PR. Before I do that, I'd like to take a step back and reconsider the need for this. What do you want to be able to do with gptel-org-property-names exactly?