Restructure Wakatime Properties and Allow Environment Variable Parsing on Properties

Code of Conduct

[X] I agree to follow this project's Code of Conduct

What would you like to see changed/added?

Hello!

This is a two feature request that revolves around the Wakatime segment. Assuming that one modifies/implements the other.

Currently, I was building my own custom theme and I stumbled upon the Wakatime segment. Upon evaluation, I feel like there's a need for adjustments.

Wakatime Properties Refactor Sugggestion

Consume the following below: (From https://ohmyposh.dev/docs/segments/wakatime#sample-configuration)

{
    "type": "wakatime",
    "style": "powerline",
    "powerline_symbol": "\uE0B0",
    "foreground": "#ffffff",
    "background": "#007acc",
    "properties": {
        "url": "https://wakatime.com/api/v1/users/current/summaries?start=today&end=today&api_key=API_KEY",
        "cache_timeout": 10,
        "http_timeout": 500
    }
},

From this JSON structure, we can see that the URL property is exposed, shouldn't it be embedded inside the engine? Also, I don't think there are other endpoints that send the summary context but other than that, I could think of this segment as an HTTP Segment that displays custom data.

Properties such as cache_timeout and http_timeout seem to be inconsistent in terms of time metric. According to documentation, http_timeout defaults to 20ms, while the cache_timeout implicates its default (10m) under the time metric of m which goes by minutes. I think they should have a consistent time metric such as milliseconds or seconds to avoid confusion.

With those insights, I think we can re-structure the wakatime segment under the following:

{
    "type": "wakatime",
    "style": "powerline",
    "powerline_symbol": "\uE0B0",
    "foreground": "#ffffff",
    "background": "#007acc",
    "properties": {
        "api_key": "XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXX",  // Also allow Env properties to be rendered here.
        "cache_timeout_seconds": 10,
        "http_timeout_seconds": 500,
        "range": "" // string, can be daily, weekly, or monthly. Achievable by evaluating `start` and `end` URL parameters and with ISO dateformat.
    }
},

The new JSON structure now properly shows [1] a clean way to insert an API_KEY, [2] time metric now consistent between timeout properties, and [3] additional attributes (highlighting start and end datetime) correlating to the summaries API URL parameters were added, allowing for more customization or query of data. (See more information here about other parameters to include in the properties: https://wakatime.com/developers#summaries)

Also, if timeout property proposal change is implausible; another alternative suggestion for [2] would be indicating the time metric as its suffix (for instance, cache_timeout -> cache_timeout_min or http_timeout -> http_timeout_ms), so that the implication of the time metric is there.

Environment Variable Parsing at Properties

As API_KEY was highlighted in the new JSON structure, another suggestion or feature that I want to see is the Environment Variable Parsing under the Properties of a segment. So far, I have never seen any string parsing example under the properties of any other segment examples and I think the introduction of env parsing would allow other themes to extend other services to display as segments such as Brewfather, OpenWeatherMap(OWM), Withings, etc, other than the WakaTime.

We need this (speaking for myself) this feature as per my statement in the first section to allow services to be rendered as segments and to be extended in themes without worrying about accidental committing changes with a personal token.

Pictures below are the attempts to fetch data with Wakatime with the Environment Variable appended in the URL string.

Upon checking the debug, it shows HTTP 401 Unauthorized.

To verify that the env var reference has a value, the figure below shows its preceding values.

To verify that the (transient) prompt was able to parse its value, the following figure/s show its declaration (as highlighted) and output.

Output:

Output concludes that properties cannot parse environment values. Other than that, I cannot suspect if sprig cannot be parsed under properties, but I think it's likely.

In the end, all of these are a suggestion and I may be asking too much at this point, feel free to be against it.

@CodexLink hey, allow me to explain a few things to share some context and take the conversation from there.

we can see that the URL property is exposed

Yes, as embedding it would potentially require exposing the query string or individual parameters. You could argue to only add the query parameters, which would be valid, but changing that now is only going to be annoying for most. And for some users also confusing. Back when this was contributed, the conversation we had was around exposing every property, or the your proposal and we went for this as the most flexible solution. Especially for such a straightforward segment.

Properties such as cache_timeout and http_timeout seem to be inconsistent in terms of time metric

Correct, the reasoning is that they're both timeout values, which isn't a unit but a purpose. While http requests traditionally are regulated on miliseconds level, this isn't the case for a cache timeout which in this case is regulated on a minutes level. Otherwise that's a lot of conversions for people. The docs should be clear on this, if not I'll gladly solve that. Adding multiple options for clear value indication isn't something I'm leaning towards, we aim to reduce the logic so we can keep the prompt fast.

another suggestion or feature that I want to see is the Environment Variable Parsing under the Properties of a segment

I like this. A lot. The reasoning makes sense. All it takes is allowing a template for that property and we're good to go:

{
    "type": "wakatime",
    "style": "powerline",
    "powerline_symbol": "\uE0B0",
    "foreground": "#ffffff",
    "background": "#007acc",
    "properties": {
        "url": "https://wakatime.com/api/v1/users/current/summaries?start=today&end=today&api_key={{ .Env.WAKATIME_API_KEY }}",
    }
},

@JanDeDobbeleer my bad for the late response, I did the issue at 3 am, anyway...

You could argue to only add the query parameters, which would be valid, but changing that now is only going to be annoying for most. And for some users also confusing.

I think that is the whole point of my restructure proposal, to un-expose the URL and take query parameters instead to make it look clean as possible and schema-correct-able (like the start and end date feed through the URL parameters). Character-wise, it makes the segment shorter.

I don't quite agree on how it will be annoying (because I don't see it yet) but I can tell that it will be confusing as heck because once applied it will be a breaking change. Changes gonna hurt but for the sake of benefits stated in the above paragraph, it will be better.

(In regards to time metric of timeout properties) | Otherwise that's a lot of conversions for people. The docs should be clear on this, if not I'll gladly solve that.

I'm not quite sure if I'm the only one, but I have this habit that when it comes to time-based properties, I would assume that it may be evaluated under milliseconds. But after looking at the docs, it states that cache_timeout would be in minutes. I think indicating just the time metric from the suffix of these properties is something that would be a clear indication.

Apart from that, just by looking at the sample configuration alone, I would be confused as to what time metric would this be in. The time it takes for some to go through the docs is the reason why I want it to be clearer.

Adding multiple options for clear value indication isn't something I'm leaning towards, we aim to reduce the logic so we can keep the prompt fast.

This is not what I think so far because it adds complexity so we left this one out as I'm also not leaning toward it, but rather a proper time indication from the suffix of these timeout properties.

I like this. A lot. The reasoning makes sense. All it takes is allowing a template for that property and we're good to go:

Glad I'm not the only one who agrees with this. 😄

rather a proper time indication from the suffix of these timeout properties

@CodexLink this can be added to the schema so you get this information in your editor. Will make things a lot less confusing when editing.

I'm implementing 3, will add this as well and I'm still thinking about 1. Maybe when I'm back from my holidays I can see what the effect is.

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues. If you have found a problem that seems similar, please open a discussion first, complete the body with all the details necessary to reproduce, and mention this issue as reference.

JanDeDobbeleer / oh-my-posh