Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Change default cache strategy to content #6477

Merged
merged 11 commits into from
Nov 4, 2024
Merged

feat: Change default cache strategy to content #6477

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8ce8e49
Change default cache strategy to `content`
kachkaev Nov 4, 2024
f1d78b2
wip
kachkaev Nov 4, 2024
da8b573
Revert .github/workflows/cspell-cli.yml
kachkaev Nov 4, 2024
f4cf3a1
Revert cspell.schema.json
kachkaev Nov 4, 2024
4a3ea2a
Update tests
kachkaev Nov 4, 2024
059f9ab
Update schema
kachkaev Nov 4, 2024
a180bc5
Revert confusing diff
kachkaev Nov 4, 2024
6ff55f0
Temporarily commit diff that helped me run build
kachkaev Nov 4, 2024
11af5a3
Revert "Temporarily commit diff that helped me run build"
kachkaev Nov 4, 2024
daf018e
Revert diff in packages/cspell/static/help-lint.txt
kachkaev Nov 4, 2024
65d6fa7
Merge branch 'main' into change-default-cache-strategy
kachkaev Nov 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/cspell-cli.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,4 @@ jobs:
cspell-cache-v2-${{ runner.os }}-

- name: cspell@latest
run: npx cspell@latest --cache --cache-strategy=content --cache-location=.cspellcache --exclude="yarn2" --no-progress "**"
# run: npx cspell@latest --cache --cache-strategy=content --cache-location=.cspellcache --exclude="yarn2" "**"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment originates from #2303. Looks like a leftover from some experiment. Removing to avoid confusion.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to not change the workflow until cspell has been release. The npx cspell@latest will run the currently published version of cspell not the one checked in.

run: npx cspell@latest --cache --cache-location=.cspellcache --exclude="yarn2" --no-progress "**"
8 changes: 4 additions & 4 deletions cspell.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@
"type": "object"
},
Jason3S marked this conversation as resolved.
Show resolved Hide resolved
"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).",
"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
8 changes: 4 additions & 4 deletions packages/cspell-types/cspell.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@
"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).",
"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
4 changes: 2 additions & 2 deletions packages/cspell-types/src/CSpellSettingsDef.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,10 @@ 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).
*/
export type CacheStrategy = 'metadata' | 'content';
export type CacheStrategy = 'content' | 'metadata';

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

Expand Down
8 changes: 4 additions & 4 deletions packages/cspell/src/app/application.test.ts
View file
Open in desktop
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
View file
Open in desktop
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
30 changes: 15 additions & 15 deletions packages/cspell/src/app/util/cache/createCache.test.ts
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/cspell/src/app/util/cache/createCache.ts
View file
Open in desktop
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
Loading
Loading