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

feat(language): allow tweaking version cache #5859

Merged
merged 2 commits into from
Nov 8, 2024
Merged

feat(language): allow tweaking version cache #5859

merged 2 commits into from
Nov 8, 2024

Conversation

JanDeDobbeleer
Copy link
Owner

@JanDeDobbeleer JanDeDobbeleer commented Nov 8, 2024

Prerequisites

  • I have read and understood the contributing guide.
  • The commit message follows the conventional commits guidelines.
  • Tests for the changes have been added (for bug fixes / features).
  • Docs have been added/updated (for bug fixes / features).

@JanDeDobbeleer
Copy link
Owner Author

JanDeDobbeleer commented Nov 8, 2024

@lewis-yeung .Text can never be taken into account for cross template properties for primary and right, only for extra prompts. The reason is that the execution happens in parallel and writing sequentially. The check looks at if the segment has been executed in case there's a need in a template, but .Text is only available after writing, and thus after that check happens, as needs is a precondition for resolving .Text. Theoretically a check could be built-in that validates the presence of .Text and its value, but that would only work sequentially and in my opinion, that logic isn't worth my while as I don't really see the use case in primary and right.

@JanDeDobbeleer JanDeDobbeleer merged commit f838eaf into main Nov 8, 2024
9 checks passed
@lewis-yeung
Copy link
Contributor

@JanDeDobbeleer Got it. I was taking that as an example to reproduce the issue, but not using it in my own config.

@JanDeDobbeleer
Copy link
Owner Author

@lewis-yeung It was a really good catch, but that should be the only case where this behavior occurs, which I can live with as "by design". I want to keep that part as simple as possible. If you notice any other behavior, let me know!

@lewis-yeung
Copy link
Contributor

@JanDeDobbeleer Unfortunately, reverting the fix brought back the issue of #2885. It occurs not only when cross-referencing the .Text of a segment, but any property.

@JanDeDobbeleer
Copy link
Owner Author

@lewis-yeung I made sure to test this with cross segment properties. I can double check, but this shouldn't be the case though.

@lewis-yeung
Copy link
Contributor

@JanDeDobbeleer It should work in an extra prompt, right? However, I always see the issue when using a cross segment property in the transient prompt, but not with the previous fix of ce02759.

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

Successfully merging this pull request may close these issues.

2 participants