-
Notifications
You must be signed in to change notification settings - Fork 526
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
weather@mockturtl Add more tags, add padding options #6455
Conversation
cc @Gr3q |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please increment the version in metadata.json
and add the new version to CHANGELOG.md
.
Edit: Looks nice, I will test it when I get home.
weather@mockturtl/src/3_8/utils.ts
Outdated
['t_h_diff', tempHourDiff], | ||
['br', "\n"] | ||
]; | ||
var regexp, match, pad, padLeft, padChar, newVal, isLiteral, padLiteral, dontPad; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use let
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's all up to date, let me know if you spot anything else, cheers :)
weather@mockturtl/src/3_8/utils.ts
Outdated
const dayLength = ToHoursMinutes(weather.sunset - weather.sunrise); | ||
const now = new Date(); | ||
const sunny = now > weather.sunrise && now < weather.sunset; | ||
const daylightRemain = sunny ? ToHoursMinutes(weather.sunset - now) : ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be incorrect if the system timezone doesn't match the locations timezone (maybe it's all ok, I'm just not sure).
Please verify by testing a location outside your timezone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested in another time zone, seems to work despite Date not containing timezone info.
weather@mockturtl/src/3_8/utils.ts
Outdated
const tempsTomorrowWithDifferences = tmr ? tempsTomorrow + " (" + tmrMinTempChange + " / " + tmrMaxTempChange + ")" : ""; | ||
const sunset = (GetHoursMinutes(weather.sunset, config._show24Hours)); | ||
const sunrise = (GetHoursMinutes(weather.sunrise, config._show24Hours)); | ||
const dayLength = ToHoursMinutes(weather.sunset - weather.sunrise); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intl.DurationFormat doesn't exist in cjs at all so I guess there is no other option and I don't want you to add another lib like humanizeDuration
so this is fine.
OK will stop adding stuff on this PR :) let me know if any changes are required, cheers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have a bunch of type and eslint errors, including things that would break support with Cinnamon 3.8. You will have to edit this in something with typescript and eslint integration.
Easiest is with vscode but you can use something else, the main thing is that you will need to open the weather@mockturtl folder directly so it uses the tsconfig.json correctly.
Or you can validate from the terminal, these should not return any errors:
npx tsc -p ./src/3_8/tsconfig.json --noEmit
npm run lint
As for code changes I would suggest something like this (I don't know what true
means in ['t', temp, 4, true],
for example, so if you use an interface it's more readable):
type InjectItem =
"t" |
"u" |
"c" |
"c_long" |
"dew_point" |
"humidity" |
"pressure" |
"pressure_unit" |
"extra_value" |
"extra_name" |
"city" |
"country" |
"search_entry" |
"last_updated" |
"wind_speed" |
"wind_dir" |
"wind_arrow" |
"wind_deg" |
"wind_unit" |
"min" |
"max" |
"tmr_min" |
"tmr_max" |
"tmr_min_diff" |
"tmr_max_diff" |
"tmr_c" |
"tmr_t" |
"tmr_td" |
"sunset" |
"sunrise" |
"day_length" |
"day_remain" |
"day_len_rem" |
"day_rem_pct" |
"t_h_diff" |
"br";
;
interface PaddingDefault {
padding?: number;
}
export const INJECT_ITEMS: Record<InjectItem, PaddingDefault> = {
"t": { padding: 4 },
"u": {},
"c": {},
"c_long": {},
"dew_point": {},
"humidity": { padding: 3 },
"pressure": { padding: 7 },
"pressure_unit": {},
"extra_value": {},
"extra_name": {},
"city": {},
"country": {},
"search_entry": {},
"last_updated": {},
"wind_speed": {},
"wind_dir": {},
"wind_arrow": {},
"wind_deg": {},
"wind_unit": {},
"min": {},
"max": {},
"tmr_min": {},
"tmr_max": {},
"tmr_min_diff": {},
"tmr_max_diff": {},
"tmr_c": {},
"tmr_t": {},
"tmr_td": {},
"sunset": {},
"sunrise": {},
"day_length": {},
"day_remain": {},
"day_len_rem": {},
"day_rem_pct": {},
"t_h_diff": {},
"br": {}
};
export function InjectValues(text: string, weather: WeatherData, config: Config, inCommand: boolean = false): string {
...
const valuesPaddingDefaults: Record<InjectItem, string> = {
't': TempToUserConfig(weather.temperature, config, false) ?? "",
'u': UnitToUnicode(config.TemperatureUnit),
'c': weather.condition.main,
'c_long': weather.condition.description,
'dew_point': TempToUserConfig(weather.dewPoint, config, false) ?? "",
'humidity': weather.humidity?.toString() ?? "",
'pressure': weather.pressure != null ? PressToUserUnits(weather.pressure, config._pressureUnit).toString() : "",
'pressure_unit': config._pressureUnit,
'extra_value': weather.extra_field ? ExtraFieldToUserUnits(weather.extra_field, config) : "",
'extra_name': weather.extra_field ? weather.extra_field.name : "",
'city': weather.location.city ?? "",
'country': weather.location.country ?? "",
'search_entry': config.CurrentLocation?.entryText ?? "",
'last_updated': AwareDateString(weather.date, config._show24Hours, DateTime.local().zoneName),
'wind_speed': weather.wind.speed != null ? MPStoUserUnits(weather.wind.speed, windUnit) : "",
'wind_dir': weather.wind.degree != null ? CompassDirectionText(weather.wind.degree) : "",
'wind_arrow': weather.wind.degree != null ? CompassDirectionArrow(weather.wind.degree) : "",
'wind_deg': weather.wind.degree != null ? weather.wind.degree.toString() : "",
...
So it's strictly typed.
I have removed all the warnings in vscode except for 1, it is complaining about using regex groups:
How do I change to es2018 or is that error misleading or ? Edit: microsoft/TypeScript#32098
|
If it helps, just a note that Cinnamon 3.8 shipped with Mint 19.0 which was in turn based on Ubuntu 18.04 which ended standard support in 2023. At this point, it would be safe to create a new target version directory going forward and leaving the 3.8 folder for everything in-between. The new version folder could be 5.4 or 6.2. All prior versions of Cinnamon would be served by the 3.8 folder (and hopefully only requiring minimal maintenance) while new features, functionality, and/or fixes can be focused on more recent versions. |
I think I will move the overrides to a new tab in the settings, and have a Tree section for padding defaults. https://github.com/linuxmint/cinnamon/blob/1e43299b9ad548c9395edfd456cbba017ca02b9f/files/usr/share/cinnamon/applets/settings-example%40cinnamon.org/settings-schema.json#L218 |
There will be a day when something comes up to abandon older versions, this is really not it. @aperkins81 if you remove named capturing groups from the regex and use regular capturing groups, it will work. Cinnamon is also available on Debian, so I will try to support 3.8 as long as it's not a huge burden (like it used to be with 3.2 and 3.6). I can use all new language features with a few exceptions, this is one of them. |
You should commit the code. And I can see you still don't handle the case if
The list setting element is the best out of all elements for what you are trying to do but it's still not the ideal, because the user can add/remove items from it and we have a fixed set items, you just want users to edit them. So either you copy the setting element from the Cinnamon source code and remove those buttons or you add code on the applet side every time to populate them (which you will need to do anyway because the list of what we support can change - expand - from version to version)
That should be it's own PR, it sound's like a big job. to do that you would need to support and evaluate actual programming logic in settings. You should create a list of all conditions and outputs you want to see in there with a priority order based on what kind of forecast is available (minutely, hourly, daily) PS: For now just omit the extra features from this PR and focus on fixing the errors |
@aperkins81 Alternatively I can open a PR on your fork where I fix all errors so it gets included in this PR, whichever you prefer. |
Whether it be 4.8 or 5.4 as a new folder, you can safely leave 3.8 as it is. Hopefully this helps! 1.Debian 11 has also transitioned support from the Debian security and release teams to the LTS team. |
Yeah will keep conditional presets for another PR and fix those other errors today.
and above:
Haven't tested that much as am dealing with another error but open to suggestions (Edit: will put this in another PR) |
Note in last commit after running ./build.sh it made a heap of changes in the generated weather-applet.js - is this normal or do I have something misconfigured? (Edit: it also changed a heap of translation files, I discarded changes of this because I did not know what it was doing, but if you would like I can generate them and add them to the commit) |
After 12 years of Ruby on Rails adhering to DRY principle, I physically cannot triplicate that list ;) I ran it through ChatGPT and it gave more descriptive variable names so hoping existing is all good |
Just a note for the next version supporting es2018 there is a linter requiring named groups for readability https://eslint.org/docs/latest/rules/prefer-named-capture-group |
You should generate and commit translations, they were changed after all.
npm ci
npm watch # this will not exit, it will watch for changes after building (if there are errors, it will not output) If you still have a lot of changes after that then it must be the nodejs/npm version difference and they can be committed. |
I understand where you are coming from, in that case
I agree named capturing groups are much more readable and I prefer them too, and I will add it as a rule when I break compatibility with older versions. It's most likely will be when anything below Cinnamon 5.4 stops being supported, or whatever version the Soup library version has changed in Cinnamon. |
weather@mockturtl/src/3_8/utils.ts
Outdated
// Sunrise and sunset calculations | ||
const sunsetTime = sunset ? GetHoursMinutes(sunset, _show24Hours) : ""; | ||
const sunriseTime = sunrise ? GetHoursMinutes(sunrise, _show24Hours) : ""; | ||
const dayLength = sunset && sunrise ? ToHoursMinutes(Number(sunset) - Number(sunrise)) : ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this safe? GetHoursMinutes
can output wildly different strings based on the user's locale. Have you tried it with german, hungarian or korean locale?
It's easiest to test if you add a line date.setLocale("yourlocale");
inside GetHoursMinutes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will get in a patch today to use the builtins, been sidetracked with housework; was initially largely unaware of the TypeScript as I developed in in JS, but another skill to learn is good, will try help out with other issues and would next like to build a text VU meter that by default uses coloured |||||| (and some nice utf8 symbols as themes) but also can use the output from a file (eg, this weather applet, then set panel text to off in this applet). PS love Cinnamon, it was the only sane option when Gnome 3 was forced on us, and back in the day I wrote a battery applet for Gnome 2. These days I have been using CachyOS for the last few months and before that Arch for a good 15 years after Ubuntu. Also I have a WIP that the robot (mostly) made, I have been using Looking Glass to debug.
#!/bin/bash
# REQUIREMENTS:
# - typescript installed
version='3.8'
TAIL_OUTPUT=false # Set to true if you want to keep the script running (tail output)
# Run the npm lint command and capture its output
lint_output=$(npm run lint 2>&1)
# Use awk and sed to extract and clean the number of errors and warnings
errors=$(echo "$lint_output" | awk '/problems/ {print $4}' | sed 's/[()]//g')
warnings=$(echo "$lint_output" | awk '/problems/ {print $6}' | sed 's/[()]//g')
echo "Errors: $errors"
echo "Warnings: $warnings"
# Check if there are zero errors
if [ "$errors" -eq 0 ]; then
SOURCE="${BASH_SOURCE[0]}"
while [ -h "$SOURCE" ]; do # resolve $SOURCE until the file is no longer a symlink
DIR="$( cd -P "$( dirname "$SOURCE" )" >/dev/null && pwd )"
SOURCE="$(readlink "$SOURCE")"
[[ $SOURCE != /* ]] && SOURCE="$DIR/$SOURCE" # if $SOURCE was a relative symlink, we need to resolve it relative to the path where the symlink file was located
done
DIR="$( cd -P "$( dirname "$SOURCE" )" >/dev/null && pwd )"
# Save current dir for convenience
path=${PWD}
# Run the npx webpack command and capture its output
webpack_output=$(npx webpack 2>&1)
echo "$webpack_output"
# Check if the word 'successfully' is in the output
if echo "$webpack_output" | grep -q "compiled successfully in"; then
cp --verbose files/weather@mockturtl/${version}/weather-applet.js ~/.local/share/cinnamon/applets/weather\@mockturtl/3.8/weather-applet.js
# Restart Cinnamon in the background without terminating the script
nohup bash -c "export DISPLAY=:0; cinnamon --replace" > /dev/null 2>&1 &
if [ "$TAIL_OUTPUT" = true ]; then
# Tail the output to keep the script running
tail -f /dev/null # Keep the script running indefinitely
else
echo "Build and installation successful, exiting."
exit 0 # Exit the script
fi
else
echo "Errors prevented compiling"
exit -2
fi
else
echo "Errors prevented compiling: $lint_output"
exit -1
fi
Will have a look at merge conflicts tomorrow (might it be easier to disbandon this PR and open a new one?) |
No description provided.