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

Add Argument for Filtering Token Types #34

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

apackin
Copy link
Contributor

@apackin apackin commented Jun 17, 2024

Addresses #32

Pull Request: Add Argument for Filtering Token Types

Overview

This pull request introduces a new argument, filteredTokenTypes, to exclude specified token types from parsed results:

  • New Argument: filteredTokenTypes
  • Functionality: Omits specified token types from processing
  • Usage: --filtered-token-types=core,source

Files Modified

  1. bin/figma2flutter.dart

    • Main Function:
      • Added filteredTokenTypes option
      • Parsed argument into a list of token types
      • Passed filteredTypes to _processTokens
    • Token Processing:
      • Implemented filtering logic in _processTokens
  2. lib/config/options.dart

    • Option Definitions:
      • Added filteredTokenTypes constants and option for configuration
  3. lib/processor.dart

    • Processor Class:
      • Updated process method to accept filteredTypes
      • Added logic to remove tokens based on filteredTypes list

Example Usage

To run the tool and exclude tokens of types core and source:

dart bin/figma2flutter.dart --filtered-token-types=core,source

This will filter out tokens with paths starting with core or source.

Conclusion

These updates provide more control over the token processing, enabling users to fine-tune their output. Please review the changes and provide your feedback.

Notes:

I tried skipping reading the "Core" tokens altogether, but other tokens rely on their processing to assign the correct values.

@frederikstonge
Copy link
Collaborator

It's a bit too specific imo. Adding a filter options to filter an array of token types would be a better solution.

You have removed a lot of comments, I don't think it was necessary.

Please add unit tests.

Please update documentation.

@apackin apackin changed the title add flag for skipping "core" tokens add an argument for filteredTokenTypes to be omitted from parsed results Jun 17, 2024
@apackin apackin changed the title add an argument for filteredTokenTypes to be omitted from parsed results Add Argument for Filtering Token Types Jun 17, 2024
@apackin
Copy link
Contributor Author

apackin commented Jun 17, 2024

Thank you for the quick comments. I made the recommended code changes. Let me know if that looks good and I'll update with tests and docs later

@frederikstonge
Copy link
Collaborator

Thank you for the quick comments. I made the recommended code changes. Let me know if that looks good and I'll update with tests and docs later

Looks good to me :)

@freemansoft
Copy link
Collaborator

Should this be filteredTokenPaths or filteredTokenSets in place of filteredTokenTypes?

This filtering does what? It filters out all the tokens that were part of a token set while letting them be used as a base value for tokens that are retained?

@apackin
Copy link
Contributor Author

apackin commented Jun 18, 2024

It filters out all the tokens that were part of a token set

Yes, that is the intended usage. Technically there isn't enforcement regarding whether the filtered out items were used as a base value for other tokens, so it could be used to omit sets for other reasons if those exist. Happy to rename to filteredTokenSets

@apackin
Copy link
Contributor Author

apackin commented Jun 21, 2024

I renamed the argument and added tests

lib/processor.dart Outdated Show resolved Hide resolved
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.

3 participants