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.

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

feat: new extractor #88

Merged
merged 4 commits into from
Jul 2, 2024
Merged

feat: new extractor #88

merged 4 commits into from
Jul 2, 2024

Conversation

stepan662
Copy link
Contributor

@stepan662 stepan662 commented Jun 9, 2024

Closes #63
Closes #87

 - improved performance
 - parsing all files not just those importing from `@tolgee/*`
 - default namespace option
 - option to not be so strict with namespace detectability
@stepan662 stepan662 marked this pull request as ready for review July 1, 2024 08:56
Copy link
Collaborator

@cyyynthia cyyynthia left a comment

Choose a reason for hiding this comment

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

That's some impressive changes! ❤️

I haven't look very deeply into the state machines logic; tests should've caught any regression while rewriting the extraction logic. Looks neat though!

src/extractor/index.ts Outdated Show resolved Hide resolved
src/extractor/parser/tokenMergers/commentsMerger.ts Outdated Show resolved Hide resolved
src/extractor/parser/tokenMergers/typesCastMerger.ts Outdated Show resolved Hide resolved
@stepan662 stepan662 merged commit f6b7fd3 into main Jul 2, 2024
4 checks passed
@stepan662 stepan662 deleted the stepangranat/new-extractor branch July 2, 2024 08:59
@OrenChapo
Copy link

Thanks for your great work Stephan!

I tried the recent version of the cli with the new extractor, and it fails on "Dynamic options" for this line:

throw this.$t(
            'dialog.importUsers.badServerResponse',
            'Invalid response from Server.\nPlease retry later.',
          );

Works OK after removing the \n.
Is this expected?

@OrenChapo
Copy link

OrenChapo commented Jul 2, 2024

Another issue, seems like the @tolgee-key comments ignore the specified --default-namespace parameter and assume no namespace.

@stepan662
Copy link
Contributor Author

@OrenChapo thanks for reporting, should be fixed in this PR:
#91

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sync command fails to detect keys from standalone functions
3 participants