JanDeDobbeleer / oh-my-posh

The most customisable and low-latency cross platform/shell prompt renderer
https://ohmyposh.dev
MIT License
17.55k stars 2.4k forks source link

feat(language): allow tweaking version cache #5859

Closed JanDeDobbeleer closed 2 weeks ago

JanDeDobbeleer commented 2 weeks ago

Prerequisites

JanDeDobbeleer commented 2 weeks ago

@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.

lewis-yeung commented 2 weeks ago

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

JanDeDobbeleer commented 2 weeks ago

@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 commented 2 weeks ago

@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 commented 2 weeks ago

@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 commented 2 weeks ago

@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 ce027593b1bc397b0b9c65749c0020c900466c00.

JanDeDobbeleer commented 2 weeks ago

@lewis-yeung ah, that's a good one. I didn't explicitly check that. It definitely works in primary and right as otherwise #5813 would be broken again. That's the one that broke when fixing #2885 previously.

JanDeDobbeleer commented 2 weeks ago

@lewis-yeung I can't reproduce this though, my transient prompt references the git segment successfully (text and properties).

lewis-yeung commented 2 weeks ago

@JanDeDobbeleer Have you checked on Windows? It happens more often on Windows.

JanDeDobbeleer commented 2 weeks ago

@lewis-yeung I only test on Windows. Can't reproduce it at all.

JanDeDobbeleer commented 2 weeks ago

@lewis-yeung care to share the config?

lewis-yeung commented 2 weeks ago

A minimal example:

version: 3
patch_pwsh_bleed: true
final_space: true
blocks:
  - type: prompt
    alignment: left
    segments:
      - type: os
        style: plain
        properties:
          windows: ' '
        template: '{{.Icon}}'
      - type: text
        style: plain
        template: foo|
      - type: text
        style: plain
        template: bar|
      - type: text
        style: plain
        template: baz|
transient_prompt:
  template: '->{{.Segments.Os.Icon}}<-'

Result:

screenshot

JanDeDobbeleer commented 2 weeks ago

@lewis-yeung I'll test after dinner. Using this extensively on my setup as well, haven't noticed that.

JanDeDobbeleer commented 2 weeks ago

@lewis-yeung I can reproduce this, but it seems to be linked to extra only, non-extra isn't impacted.

JanDeDobbeleer commented 2 weeks ago

@lewis-yeung so, the issue is with the key value map that's behind the concurrent map. Everything gets added, but not always correctly. Sometimes there seems to be issues in concurrency, even when that should not happen at all. And when it happens, the cache writes an empty map, my assumption is that the memory layout somehow breaks.