BirjuVachhani / spider

A small dart library to generate Assets dart code from assets folder.
https://spider.birju.dev/
Apache License 2.0
190 stars 19 forks source link

Feature/add subgroups #50

Closed Sanlovty closed 2 years ago

Sanlovty commented 2 years ago

Description of the Change

Add a subgroup system to allow the user to create one class that includes different asset groups with unique file prefixes, paths, and extensions.

Benefits

Now you can create more flexible aseet-classes by providing different paths, file extensions and prefixes into the same class:

Config:

  generate_tests: true
  no_comments: true
  export: true
  use_part_of: true
  use_references_list: false
  package: resources
  groups:
    - class_name: Assets
      subgroups:
        - path: assets/images
          types: [ jpg ]
          prefix: jpg
        - paths: 
          - assets/images
          - assets/imgs
          types: [ png ]
          prefix: png

Result:

part of 'resources.dart';

class Assets{
  Assets ._();

  static const String jpgTest2 = 'assets/images/test2.jpg';
  static const String pngTest1 = 'assets/images/test1.png';
  static const String pngTest3 = 'assets/imgs/test3.png';
}

Possible Drawbacks

Verification Process

What process did you follow to verify that your change has the desired effects?

Applicable Issues

49

BirjuVachhani commented 2 years ago

@Sanlovty Let's not add a dependency to tuple if you're only using Tuple2. We can have a local class model that makes more sense!

Sanlovty commented 2 years ago

@Sanlovty Let's not add a dependency to tuple if you're only using Tuple2. We can have a local class model that makes more sense!

Sure, i'll make it <3

Sanlovty commented 2 years ago

@BirjuVachhani, can u assign me to pr?

BirjuVachhani commented 2 years ago

@Sanlovty When I said Replace tuple with something more meaningful I meant something like this!

class SubgroupProperty {
  final String prefix;
  final Map<String, String> files;
}

Its fine if you don't have time for now to do this or if you don't want to. I can always take a look later.

Sanlovty commented 2 years ago

Its fine if you don't have time for now to do this or if you don't want to. I can always take a look later.

Oh, my bad. I'll fix it. I have time and i want to do it.

BirjuVachhani commented 2 years ago

@Sanlovty Thank you! Appreciate that! 😊

Sanlovty commented 2 years ago

@BirjuVachhani, I think it’s working as you want now. I want to add effective tests to touch each case, but I also need to rewrite some of the existing ones. I’m waiting for your feedback.

Example of using:

generate_tests: true
no_comments: true
export: true
use_part_of: true
use_references_list: false
package: resources

groups:
  - class_name: Images
    path: assets/images
    types: [ .png, .jpg, .jpeg, .webp, .webm, .bmp ]

  - class_name: Svgs
    sub_groups:
      - path: assets/svgsMenu
        prefix: menu
        types: [ .svg ]

      - path: assets/svgsOther
        prefix: other
        types: [ .svg ]

  - class_name: Ico
    types: [ .ico ]
    prefix: ico
    sub_groups:
      - path: assets/icons
        prefix: test1
        types: [ .ttf ]

      - path: assets/vectors
        prefix: test2
        types: [ .pdf ]

  - class_name: Video
    types: [ .mp4 ]
    path: assets/moviesOnly
    sub_groups:
      - path: assets/movies
        prefix: common

      - path: assets/moviesExtra
        prefix: extra

I am online from 7:00 to 22:00 (GMT)

BirjuVachhani commented 2 years ago

@Sanlovty Over all look good. Nice work! 👍🏻 Just some minor changes and tests, other than that, it should be good to merge.

Sanlovty commented 2 years ago

@BirjuVachhani if u agree with commits after cd55a39, than i start making the tests. Also, pls check email ^-^

I will appreciate if u suggest the naming ideas for vars\getters. I also was thinking about not making testConfigs 'getters' but we easily can change it if needed

BirjuVachhani commented 2 years ago

@Sanlovty Those commits look good. I like names you have given to var/getters.

I also was thinking about not making testConfigs 'getters' but we easily can change it if needed

Agreed. I am fine either way.

codecov-commenter commented 2 years ago

Codecov Report

Merging #50 (a8a1887) into main (4e4cc97) will increase coverage by 2.26%. The diff coverage is 89.43%.

@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
+ Coverage   68.30%   70.56%   +2.26%     
==========================================
  Files          13       15       +2     
  Lines         489      564      +75     
==========================================
+ Hits          334      398      +64     
- Misses        155      166      +11     
Impacted Files Coverage Δ
lib/src/constants.dart 50.00% <ø> (ø)
lib/spider.dart 40.62% <33.33%> (+0.94%) :arrow_up:
lib/src/asset_subgroup.dart 81.81% <81.81%> (ø)
lib/src/asset_group.dart 83.87% <86.95%> (+0.53%) :arrow_up:
lib/src/dart_class_generator.dart 91.87% <92.10%> (-2.57%) :arrow_down:
lib/src/utils.dart 78.74% <92.30%> (+1.54%) :arrow_up:
lib/src/data/test_template.dart 80.00% <100.00%> (+13.33%) :arrow_up:
lib/src/subgroup_property.dart 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 537e608...a8a1887. Read the comment docs.

BirjuVachhani commented 2 years ago

Thank you @Sanlovty for your contributions. 😊

BirjuVachhani commented 2 years ago

Released in v3.2.0