-
Notifications
You must be signed in to change notification settings - Fork 103
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
[DRAFT] Update dependencies, add prettier/eslint for consistent style, add more type safety #127
base: master
Are you sure you want to change the base?
Conversation
charset = utf-8 | ||
end_of_line = lf | ||
insert_final_newline = true | ||
indent_style = space |
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 got warnings of mixed tabs and spaces => changed this to spaces
@@ -1,2 +1,2 @@ | |||
npm node_modules |
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 this was wrong, there is no build
directory and npm node_modules
doesn't make sense
import { Plugin } from 'obsidian' | ||
import HeatmapCalendarSettingsTab from 'settings' | ||
|
||
declare global { |
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.
adding the global declare removes the need for ts-ignore when accessing the window object
|
||
const intensities = calEntries | ||
.map((e: Entry) => e.intensity) | ||
.filter((intensity): intensity is number => intensity !== undefined) |
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 is a neat trick - to type filters for undefined. intensities
with this has type number[]
inferred by the compiler
parent: heatmapCalendarGraphDiv, | ||
}) | ||
|
||
const months = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec'] |
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.
simplifies the previous duplication
} | ||
|
||
async onload(): Promise<void> { | ||
const settings = await this.loadSettings() |
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.
settings is only used here - no need for a global variable.
@@ -80,52 +47,54 @@ export default class HeatmapCalendar extends Plugin { | |||
return this.clamp(mapped, outMin, outMax) | |||
} | |||
|
|||
getWeekdayShort(dayNumber: number): string { | |||
return new Date(1970, 0, dayNumber + this.settings.weekStartDay + 4).toLocaleDateString('en-US', { | |||
getWeekdayShort(weekStartDay: number, dayNumber: number): string { |
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.
add parameter to avoid accessing a global variable
export default class HeatmapCalendar extends Plugin { | ||
settings: CalendarSettings |
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.
removing this since it is not needed, and this way we can enable strictPropertyInitialization in tsconfig
I noticed the dependencies were a bit out of date and the style was not very consistent. As well as a few
as ...
andts-ignore
.I moved the ts file into a src folder and extracted default values and the domain model.
I ran the linter & formatter on the codebase, I tried to make it do as little changes as possible but the mix of commas/semicolons/quotes needed some changes to be unified.
the formatting & linting is cnotained in a single commit so it can be reverted more easily and review of unrelated content is easier.