-
-
Notifications
You must be signed in to change notification settings - Fork 52
feat: support langs option in find command #139
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
base: master
Are you sure you want to change the base?
feat: support langs option in find command #139
Conversation
@kdraba Can you please update the branch with master so we can push this forward? 😃 |
…i_command_should_respect_langs_option # Conflicts: # src/keys-detective/compare-keys-to-files.ts
Done. |
@@ -52,25 +59,25 @@ export function compareKeysToFiles({ | |||
const scopePaths = getGlobalConfig().scopePathMap || {}; | |||
for (const [scope, path] of Object.entries(scopePaths)) { | |||
const keys = scopeToKeys[scope]; | |||
if (keys) { | |||
if (keys && typeof path === '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.
Why is this needed? path is supposed to be a string
import { resolveConfig } from '../utils/resolve-config'; | ||
|
||
import { compareKeysToFiles } from './compare-keys-to-files'; | ||
import { getTranslationFilesPath } from './get-translation-files-path'; | ||
|
||
export function findMissingKeys(inlineConfig: Config) { | ||
const logger = getLogger(); | ||
const config = resolveConfig(inlineConfig); | ||
const config = resolveConfig({ langs:[], ...inlineConfig }); |
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.
The only thing is that this config property has 2 different purposes when running the find command and when running the extract.
we need to make it clear in the docs if we want to use the same property.
Also, the default config should be different based on what action you are running, otherwise, you need to override it locally like you did here but I'm not sure that's a good practice in the long run.
@@ -24,6 +24,22 @@ interface Options extends Pick<Config, 'fileFormat' | 'translationsPath'> { | |||
filePath: string; | |||
} | |||
|
|||
export function filterPathByLang( |
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 add an explanation to why this was created and what is this used for
@@ -89,7 +96,8 @@ export function compareKeysToFiles({ | |||
...res, | |||
files: normalizedGlob( | |||
`${res.baseFilesPath}/${isGlobal ? '' : scope}/*.${fileFormat}` | |||
), | |||
) | |||
.filter(filterPathByLang(langsToProcess, scopeAndLangFromPathOption)), |
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.
Seems like you can create this filter once and use it across the function, generally speaking you can create it once with the command config no?
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #138 138
What is the new behavior?
The find command respects the langs option and only processes files for the specified languages. If the langs option is not present, all files will be processed.
Does this PR introduce a breaking change?
Usages of the find command, that specify languages using the langs option will behave differently. Instead of processing all existing file, only those files specified in the langs options will be processed.
Migration: If you want to process all files, remove the langs option from the find command invocation.
Other information