Skip to content

Commit

Permalink
Improve code docs and tests (#885)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcusolsson authored May 31, 2024
1 parent c2fb474 commit a103776
Show file tree
Hide file tree
Showing 12 changed files with 200 additions and 75 deletions.
13 changes: 10 additions & 3 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,14 @@
"env": {
"node": true
},
"plugins": ["@typescript-eslint"],
"extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"],
"plugins": [
"@typescript-eslint",
"eslint-plugin-tsdoc"
],
"extends": [
"eslint:recommended",
"plugin:@typescript-eslint/recommended"
],
"parserOptions": {
"sourceType": "module"
},
Expand All @@ -15,6 +21,7 @@
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/no-empty-function": "off",
"@typescript-eslint/no-unused-vars": "off",
"no-useless-escape": "off"
"no-useless-escape": "off",
"tsdoc/syntax": "warn"
}
}
66 changes: 66 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"esbuild-plugin-replace": "^1.4.0",
"esbuild-svelte": "^0.7.4",
"eslint": "^8.57.0",
"eslint-plugin-tsdoc": "^0.3.0",
"fp-ts": "^2.16.6",
"i18next": "^21.10.0",
"immer": "^9.0.21",
Expand Down
12 changes: 6 additions & 6 deletions src/lib/datasources/folder/datasource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ export class FolderDataSource extends FrontMatterDataSource {
*
* Assumes both folder path and file path have been normalized.
*
* @param folderPath path to the root folder, e.g. Work
* @param filePath path to the file to test, e.g. Work/Untitled.md
* @returns
* @param folderPath - The path to the root folder, e.g. Work
* @param filePath - The path to the file to test, e.g. Work/Untitled.md
* @returns True if the folder contains the given file, else false
*/
function folderContainsPath(folderPath: string, filePath: string): boolean {
const fileElements = filePath.split("/").slice(0, -1);
Expand All @@ -70,9 +70,9 @@ function folderContainsPath(folderPath: string, filePath: string): boolean {
*
* Assumes both folder path and file path have been normalized.
*
* @param folderPath path to the root folder, e.g. Work
* @param filePath path to the file to test, e.g. Work/Meetings/Untitled.md
* @returns
* @param folderPath - The path to the root folder, e.g. Work
* @param filePath - The path to the file to test, e.g. Work/Meetings/Untitled.md
* @returns True if the file exists in any subfolder.
*/
function folderContainsDeepPath(folderPath: string, filePath: string): boolean {
const fileElements = filePath.split("/").filter((el) => el);
Expand Down
47 changes: 26 additions & 21 deletions src/lib/datasources/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, it } from "@jest/globals";
import { test, describe, expect, it } from "@jest/globals";
import {
DataFieldType,
type DataField,
Expand Down Expand Up @@ -148,28 +148,33 @@ describe("detectFields", () => {
});

describe("detectCellType", () => {
it("detects simple data types", () => {
expect(detectCellType("My value")).toStrictEqual(DataFieldType.String);
expect(detectCellType(12.0)).toStrictEqual(DataFieldType.Number);
expect(detectCellType(true)).toStrictEqual(DataFieldType.Boolean);
});

it("detects repeated field types", () => {
expect(detectCellType(["foo", "bar"])).toStrictEqual(DataFieldType.String);
expect(detectCellType([1, 2])).toStrictEqual(DataFieldType.Number);
const cases: [unknown, DataFieldType][] = [
// Primitive values.
[null, DataFieldType.Unknown],
[undefined, DataFieldType.Unknown],
["My value", DataFieldType.String],
["", DataFieldType.String],
[12.0, DataFieldType.Number],
[0, DataFieldType.Number],
[true, DataFieldType.Boolean],
[false, DataFieldType.Boolean],

// Fallback to String field
expect(detectCellType([true, 1])).toStrictEqual(DataFieldType.String);
});
// Repeated values.
[["foo", "bar"], DataFieldType.String],
[[null, "bar"], DataFieldType.String],
[[1, 2], DataFieldType.Number],
[[1, null], DataFieldType.Number],
[[true, false], DataFieldType.Boolean],
[[null, false], DataFieldType.Boolean],
[[true, 1], DataFieldType.String], // Fall back to String field.
[[], DataFieldType.String], // Current behavior, but is this what we want?

it("detects null fields", () => {
expect(detectCellType(null)).toStrictEqual(DataFieldType.Unknown);
});
// Complex values.
["2022-01-01", DataFieldType.Date],
[{ my: "object" }, DataFieldType.Unknown],
];

it("detects complex field types", () => {
expect(detectCellType("2022-01-01")).toStrictEqual(DataFieldType.Date);
expect(detectCellType({ my: "object" })).toStrictEqual(
DataFieldType.Unknown
);
test.each(cases)("%s should be detected as %s", (value, expected) => {
expect(detectCellType(value)).toStrictEqual(expected);
});
});
28 changes: 18 additions & 10 deletions src/lib/datasources/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,16 @@ import {
} from "../dataframe/dataframe";

/**
* parseRecords parses the values for each record based on the detected field
* types.
* Parses the values for each record based on the detected field types.
*
* If field types matches with the corresponding data types in the records,
* this function does nothing.
*
* In the case where a field contains more than one data type, this function
* tries to parse the value in each record to match the field type.
*
* For example, if the record contains { "weight": 12 }, and the field type is
* DataFieldType.String, the resulting record has { "weight": "12"}.
* For example, if the record contains \{ "weight": 12 \}, and the field type is
* DataFieldType.String, the resulting record has \{ "weight": "12" \}.
*/
export function parseRecords(
records: DataRecord[],
Expand Down Expand Up @@ -59,9 +58,9 @@ export function parseRecords(
/**
* Merges a new version of `values` into a copy of data record.
*
* @param {Readonly<DataRecord>} record - the original data record
* @param {Readonly<DataRecord["values"]>} values - the values to merge into the original record
* @return {DataRecord} a new data record with the merged values
* @param record - The original data record
* @param values - The values to merge into the original record
* @returns A new data record with the merged values
*/
export function updateRecordValues(
record: Readonly<DataRecord>,
Expand Down Expand Up @@ -144,8 +143,14 @@ export function detectCellType(value: unknown): DataFieldType {
return DataFieldType.Unknown;
}

function stringToBoolean(stringValue: string): boolean {
switch (stringValue?.toLowerCase()?.trim()) {
/**
* Converts a string to a boolean.
*
* @param str - The string to convert.
* @returns The boolean representation of the string.
*/
function stringToBoolean(str: string): boolean {
switch (str?.toLowerCase()?.trim()) {
case "true":
case "yes":
case "1":
Expand All @@ -159,10 +164,13 @@ function stringToBoolean(stringValue: string): boolean {
return false;

default:
return !!stringValue;
return !!str;
}
}

/**
* Thrown to avoid processing more files than the plugin can handle.
*/
export class TooManyNotesError extends Error {
constructor(n: number, limit: number) {
const message = `This project contains ${Intl.NumberFormat().format(
Expand Down
11 changes: 6 additions & 5 deletions src/lib/datasources/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,25 @@ export abstract class DataSource {
) {}

/**
* queryAll returns a DataFrame with all records in the project.
* Returns a DataFrame with all records in the project.
*/
abstract queryAll(): Promise<DataFrame>;

/**
* queryOne returns a DataFrame with a single record for the given file.
* Returns a DataFrame with a single record for the given file.
*
* @param fields contains existing fields, to be able to parse file into the existing schema.
* @param fields - The existing fields to allow parsing file into the existing schema
* @returns A dataframe containing a single record
*/
abstract queryOne(file: IFile, fields: DataField[]): Promise<DataFrame>;

/**
* includes returns whether a path belongs to the current project.
* Returns whether a path belongs to the current project.
*/
abstract includes(path: string): boolean;

/**
* readonly returns whether the data source is read-only.
* Returns whether the data source is read-only.
*
* Read-only data sources are typically derived records where the data
* source can't determine the original names of the fields.
Expand Down
Loading

0 comments on commit a103776

Please sign in to comment.