flutterfire_cli icon indicating copy to clipboard operation
flutterfire_cli copied to clipboard

fix: Support spaces in paths and product name for iOS

Open AhmedLSayed9 opened this issue 1 year ago โ€ข 2 comments

Description

If the the app's directory contains space i.e: /Users/Me/With Space/MyApp, current configuration will read the path as /Users/Me and that will cause the configuration to fail when trying to access paths as /Users/Me/firebase.json.

Same issue will happen with product name, so we can't name an application with a space.

Type of Change

  • [ ] โœจ feat -- New feature (non-breaking change which adds functionality)
  • [x] ๐Ÿ› ๏ธ fix -- Bug fix (non-breaking change which fixes an issue)
  • [ ] โŒ ! -- Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] ๐Ÿงน refactor -- Code refactor
  • [ ] โœ… ci -- Build configuration change
  • [ ] ๐Ÿ“ docs -- Documentation
  • [ ] ๐Ÿ—‘๏ธ chore -- Chore

AhmedLSayed9 avatar Nov 12 '23 13:11 AhmedLSayed9

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Nov 12 '23 13:11 CLAassistant

Hey @AhmedLSayed9 - if you want to resolve the file conflict, I would look to get this merged (providing it passes integration tests) ๐Ÿ‘

russellwheatley avatar Apr 05 '24 11:04 russellwheatley

@russellwheatley Can you check now?

AhmedLSayed9 avatar Apr 10 '24 19:04 AhmedLSayed9

@AhmedLSayed9 - it isn't going to work, the script now sees those Xcode env variables as Dart variables. Look at the results of the CI run for more information.

russellwheatley avatar Apr 11 '24 07:04 russellwheatley

@AhmedLSayed9 - it isn't going to work, the script now sees those Xcode env variables as Dart variables. Look at the results of the CI run for more information.

My bad, I had to escape the '$'. I've fixed them now.

AhmedLSayed9 avatar Apr 11 '24 19:04 AhmedLSayed9

@AhmedLSayed9 - could you write a test like this: https://github.com/invertase/flutterfire_cli/blob/main/packages/flutterfire_cli/test/configure_test.dart#L342-L379

Except change the applePath to one with spaces in it and create a different one for macOS and iOS. Just select platforms as iOS and macOS in the flutterfire configure command and remove checks unrelated to apple platforms.

russellwheatley avatar Apr 12 '24 06:04 russellwheatley

@russellwheatley Thanks for the tips. I've added a test case.

AhmedLSayed9 avatar Apr 12 '24 23:04 AhmedLSayed9

This should also fix #243.

@russellwheatley Can you check if there's still something to add?

AhmedLSayed9 avatar Apr 15 '24 16:04 AhmedLSayed9

@AhmedLSayed9 - thanks for the PR. CI won't run correctly for contributor pull requests. I just tested this locally and it was successful: Screenshot 2024-04-18 at 11 34 29

Good work ๐Ÿ’ช

russellwheatley avatar Apr 18 '24 10:04 russellwheatley

@russellwheatley This fixes a critical error, shall we have a release for this ? I also faced the same issue.

rahulraj-idt avatar May 07 '24 12:05 rahulraj-idt

@rahulraj-idt - This has come out in a dev release: https://github.com/invertase/flutterfire_cli/blob/main/CHANGELOG.md#flutterfire_cli---v101-dev0

russellwheatley avatar May 10 '24 13:05 russellwheatley