Skip to content

Commit

Permalink
feat: Change default cache strategy to content (#6477)
Browse files Browse the repository at this point in the history
  • Loading branch information
kachkaev authored Nov 4, 2024
1 parent ff88b91 commit 12edd2b
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 39 deletions.
9 changes: 5 additions & 4 deletions cspell.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@
"type": "object"
},
"CacheStrategy": {
"description": "The Strategy to use to detect if a file has changed.\n- `metadata` - uses the file system timestamp and size to detect changes (fastest).\n- `content` - uses a hash of the file content to check file changes (slower - more accurate).",
"default": "content",
"description": "The Strategy to use to detect if a file has changed.\n- `content` - uses a hash of the file content to check file changes (slower - more accurate).\n- `metadata` - uses the file system timestamp and size to detect changes (fastest, may not work in CI).",
"enum": [
"metadata",
"content"
"content",
"metadata"
],
"markdownDescription": "The Strategy to use to detect if a file has changed.\n- `metadata` - uses the file system timestamp and size to detect changes (fastest).\n- `content` - uses a hash of the file content to check file changes (slower - more accurate).",
"markdownDescription": "The Strategy to use to detect if a file has changed.\n- `content` - uses a hash of the file content to check file changes (slower - more accurate).\n- `metadata` - uses the file system timestamp and size to detect changes (fastest, may not work in CI).",
"type": "string"
},
"CharacterSet": {
Expand Down
9 changes: 5 additions & 4 deletions packages/cspell-types/cspell.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@
"type": "object"
},
"CacheStrategy": {
"description": "The Strategy to use to detect if a file has changed.\n- `metadata` - uses the file system timestamp and size to detect changes (fastest).\n- `content` - uses a hash of the file content to check file changes (slower - more accurate).",
"default": "content",
"description": "The Strategy to use to detect if a file has changed.\n- `content` - uses a hash of the file content to check file changes (slower - more accurate).\n- `metadata` - uses the file system timestamp and size to detect changes (fastest, may not work in CI).",
"enum": [
"metadata",
"content"
"content",
"metadata"
],
"markdownDescription": "The Strategy to use to detect if a file has changed.\n- `metadata` - uses the file system timestamp and size to detect changes (fastest).\n- `content` - uses a hash of the file content to check file changes (slower - more accurate).",
"markdownDescription": "The Strategy to use to detect if a file has changed.\n- `content` - uses a hash of the file content to check file changes (slower - more accurate).\n- `metadata` - uses the file system timestamp and size to detect changes (fastest, may not work in CI).",
"type": "string"
},
"CharacterSet": {
Expand Down
5 changes: 3 additions & 2 deletions packages/cspell-types/src/CSpellSettingsDef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,11 @@ export interface PnPSettings {

/**
* The Strategy to use to detect if a file has changed.
* - `metadata` - uses the file system timestamp and size to detect changes (fastest).
* - `content` - uses a hash of the file content to check file changes (slower - more accurate).
* - `metadata` - uses the file system timestamp and size to detect changes (fastest, may not work in CI).
* @default 'content'
*/
export type CacheStrategy = 'metadata' | 'content';
export type CacheStrategy = 'content' | 'metadata';

export type CacheFormat = 'legacy' | 'universal';

Expand Down
12 changes: 8 additions & 4 deletions packages/cspell/src/app/__snapshots__/app.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ exports[`Validate cli > app 'lint --help --issue-template' Expect Error: 'output
" --no-cache Do not use cache.",
" --cache-reset Reset the cache file.",
" --cache-strategy <strategy> Strategy to use for detecting changed files.",
" (choices: "metadata", "content")",
" (choices: "content", "metadata", default:",
" "content")",
" --cache-location <path> Path to the cache file or directory. (default:",
" ".cspellcache")",
" --dot Include files and directories starting with \`.\`",
Expand Down Expand Up @@ -796,7 +797,8 @@ exports[`Validate cli > app 'lint --help --verbose' Expect Error: 'outputHelp' 1
" --no-cache Do not use cache.",
" --cache-reset Reset the cache file.",
" --cache-strategy <strategy> Strategy to use for detecting changed files.",
" (choices: "metadata", "content")",
" (choices: "content", "metadata", default:",
" "content")",
" --cache-location <path> Path to the cache file or directory. (default:",
" ".cspellcache")",
" --dot Include files and directories starting with \`.\`",
Expand Down Expand Up @@ -932,7 +934,8 @@ exports[`Validate cli > app 'lint --help' Expect Error: 'outputHelp' 1`] = `
" --no-cache Do not use cache.",
" --cache-reset Reset the cache file.",
" --cache-strategy <strategy> Strategy to use for detecting changed files.",
" (choices: "metadata", "content")",
" (choices: "content", "metadata", default:",
" "content")",
" --cache-location <path> Path to the cache file or directory. (default:",
" ".cspellcache")",
" --dot Include files and directories starting with \`.\`",
Expand Down Expand Up @@ -1066,7 +1069,8 @@ exports[`Validate cli > app 'no-args' Expect Error: 'outputHelp' 1`] = `
" --no-cache Do not use cache.",
" --cache-reset Reset the cache file.",
" --cache-strategy <strategy> Strategy to use for detecting changed files.",
" (choices: "metadata", "content")",
" (choices: "content", "metadata", default:",
" "content")",
" --cache-location <path> Path to the cache file or directory. (default:",
" ".cspellcache")",
" --dot Include files and directories starting with \`.\`",
Expand Down
8 changes: 4 additions & 4 deletions packages/cspell/src/app/application.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,15 +239,15 @@ describe('Linter File Caching', () => {

const NoCache: LinterOptions = { cache: false };
const Config: LinterOptions = { cacheFormat: 'legacy' };
const WithCache: LinterOptions = { cache: true, cacheStrategy: 'metadata', cacheFormat: 'legacy' };
const WithCache: LinterOptions = { cache: true, cacheStrategy: 'content', cacheFormat: 'legacy' };
// const WithCacheUniversal: LinterOptions = { cache: true, cacheStrategy: 'metadata' };
const WithCacheReset: LinterOptions = {
cache: true,
cacheStrategy: 'metadata',
cacheStrategy: 'content',
cacheReset: true,
cacheFormat: 'legacy',
};
const CacheContent: LinterOptions = { cache: true, cacheStrategy: 'content', cacheFormat: 'legacy' };
const CacheMetadata: LinterOptions = { cache: true, cacheStrategy: 'metadata', cacheFormat: 'legacy' };

test.each`
runs | root | comment
Expand All @@ -257,7 +257,7 @@ describe('Linter File Caching', () => {
${[run(['*.md'], WithCache, fc(1, 0)), run(['*.md'], WithCache, fc(1, 1)), run(['*.md'], WithCache, fc(1, 1))]} | ${fr('cached')} | ${'Single .md file cached three runs'}
${[run(['*.md'], WithCache, fc(1, 0)), run(['*.{md,ts}'], WithCache, fc(2, 1)), run(['*.{md,ts}'], WithCache, fc(2, 2))]} | ${fr('cached')} | ${'cached changing glob three runs'}
${[run(['*.md'], WithCache, fc(1, 0)), run(['*.{md,ts}'], WithCache, fc(2, 1)), run(['*.{md,ts}'], WithCacheReset, fc(2, 0))]} | ${fr('cached')} | ${'cached changing glob three runs'}
${[run(['*.md'], WithCache, fc(1, 0)), run(['*.{md,ts}'], WithCache, fc(2, 1)), run(['*.{md,ts}'], CacheContent, fc(2, 0))]} | ${fr('cached')} | ${'with cache rebuild'}
${[run(['*.md'], WithCache, fc(1, 0)), run(['*.{md,ts}'], WithCache, fc(2, 1)), run(['*.{md,ts}'], CacheMetadata, fc(2, 0))]} | ${fr('cached')} | ${'with cache rebuild'}
${[run(['*.md'], WithCache, fc(1, 0)), run(['*.{md,ts}'], WithCacheReset, fc(2, 0)), run(['*.{md,ts}'], WithCache, fc(2, 2))]} | ${fr('cached')} | ${'cached changing glob three runs'}
`('lint caching with $root $comment', async ({ runs, root }: TestCase) => {
const reporter = new InMemoryReporter();
Expand Down
7 changes: 3 additions & 4 deletions packages/cspell/src/app/commandLint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,9 @@ export function commandLint(prog: Command): Command {
.option('--no-cache', 'Do not use cache.')
.option('--cache-reset', 'Reset the cache file.')
.addOption(
crOpt('--cache-strategy <strategy>', 'Strategy to use for detecting changed files.').choices([
'metadata',
'content',
]),
crOpt('--cache-strategy <strategy>', 'Strategy to use for detecting changed files.')
.choices(['content', 'metadata'])
.default('content'),
)
.option(
'--cache-location <path>',
Expand Down
32 changes: 16 additions & 16 deletions packages/cspell/src/app/util/cache/createCache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,21 @@ const F = false;

describe('Validate calcCacheSettings', () => {
test.each`
config | options | root | expected | comment
${{}} | ${{}} | ${process.cwd()} | ${cco()} | ${''}
${{}} | ${{}} | ${__dirname} | ${cco(F, r(__dirname, '.cspellcache'))} | ${''}
${{}} | ${{}} | ${'.'} | ${cco()} | ${''}
${{}} | ${co()} | ${'.'} | ${cco()} | ${''}
${{}} | ${co(U, '.')} | ${'.'} | ${cco()} | ${'Location is a directory'}
${{}} | ${co(U, __filename)} | ${'.'} | ${cco(F, __filename)} | ${'Location is a file'}
${cs(T)} | ${co()} | ${'.'} | ${cco(T)} | ${'Use cache in config but not command-line'}
${cs(F)} | ${co()} | ${'.'} | ${cco(F)} | ${'cfg: true, cli: -'}
${cs(F)} | ${co(T)} | ${'.'} | ${cco(T)} | ${'cfg: false, cli: true'}
${{}} | ${co(T)} | ${'.'} | ${cco(T)} | ${'cfg: -, cli: true'}
${{}} | ${{ cacheStrategy: 'content' }} | ${'.'} | ${cco(F, U, 'content')} | ${'override default strategy'}
${{}} | ${co(T, U, 'content')} | ${'.'} | ${cco(T, U, 'content')} | ${'override strategy'}
${cs(U, U, 'content')} | ${co(T, U, 'metadata')} | ${'.'} | ${cco(T, U, 'metadata')} | ${'override config strategy'}
${cs(T, U, 'content')} | ${{ version }} | ${'.'} | ${cco(T, U, 'content')} | ${'override default strategy'}
config | options | root | expected | comment
${{}} | ${{}} | ${process.cwd()} | ${cco()} | ${''}
${{}} | ${{}} | ${__dirname} | ${cco(F, r(__dirname, '.cspellcache'))} | ${''}
${{}} | ${{}} | ${'.'} | ${cco()} | ${''}
${{}} | ${co()} | ${'.'} | ${cco()} | ${''}
${{}} | ${co(U, '.')} | ${'.'} | ${cco()} | ${'Location is a directory'}
${{}} | ${co(U, __filename)} | ${'.'} | ${cco(F, __filename)} | ${'Location is a file'}
${cs(T)} | ${co()} | ${'.'} | ${cco(T)} | ${'Use cache in config but not command-line'}
${cs(F)} | ${co()} | ${'.'} | ${cco(F)} | ${'cfg: true, cli: -'}
${cs(F)} | ${co(T)} | ${'.'} | ${cco(T)} | ${'cfg: false, cli: true'}
${{}} | ${co(T)} | ${'.'} | ${cco(T)} | ${'cfg: -, cli: true'}
${{}} | ${{ cacheStrategy: 'metadata' }} | ${'.'} | ${cco(F, U, 'metadata')} | ${'override default strategy'}
${{}} | ${co(T, U, 'metadata')} | ${'.'} | ${cco(T, U, 'metadata')} | ${'override strategy'}
${cs(U, U, 'metadata')} | ${co(T, U, 'content')} | ${'.'} | ${cco(T, U, 'content')} | ${'override config strategy'}
${cs(T, U, 'metadata')} | ${{ version }} | ${'.'} | ${cco(T, U, 'metadata')} | ${'override default strategy'}
`('calcCacheSettings $comment - $config $options $root', async ({ config, options, root, expected }) => {
if (!options.version) {
options.version = version;
Expand Down Expand Up @@ -90,7 +90,7 @@ describe('validate normalizeVersion', () => {
function cco(
useCache = false,
cacheLocation = '.cspellcache',
cacheStrategy: CreateCacheSettings['cacheStrategy'] = 'metadata',
cacheStrategy: CreateCacheSettings['cacheStrategy'] = 'content',
cacheFormat: CreateCacheSettings['cacheFormat'] = 'universal',
): CreateCacheSettings {
if (cacheLocation) {
Expand Down
2 changes: 1 addition & 1 deletion packages/cspell/src/app/util/cache/createCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export async function calcCacheSettings(
path.resolve(root, cacheOptions.cacheLocation ?? cs.cacheLocation ?? DEFAULT_CACHE_LOCATION),
);

const cacheStrategy = cacheOptions.cacheStrategy ?? cs.cacheStrategy ?? 'metadata';
const cacheStrategy = cacheOptions.cacheStrategy ?? cs.cacheStrategy ?? 'content';
const cacheFormat = cacheOptions.cacheFormat ?? cs.cacheFormat ?? 'universal';
const optionals: Partial<CreateCacheSettings> = {};
if (cacheOptions.cacheReset) {
Expand Down

0 comments on commit 12edd2b

Please sign in to comment.