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

Misc file tweaks #329

Closed
wants to merge 9 commits into from
Closed

Misc file tweaks #329

wants to merge 9 commits into from

Conversation

TwitchBronBron
Copy link
Member

@TwitchBronBron TwitchBronBron commented Feb 22, 2021

Here are some internal changes that I've been wanting to make for a while. These are breaking changes, but I'd rather get them in now before more projects start to depend on brighterscript and would have a more wide-reaching impact.

Notable changes:

  • rename all instances of pathAbsolute to srcPath for brevity and clarity
  • pkgPath strings now contain an actual pkg path (i.e. "source/main.brs" is now "pkg:/source/main.brs")
  • pkgPath originally stored the platform-specific path separator (\ on windows, / on everything else). Now pkgPath always stores the unix separator because that's what is used on the roku device.
  • combine some of the program file functions to auto-detect the path type and use the correct file collection (this.files or this.pkgMap) accordingly.
  • program.files and program.pkgMap now store their keys in all lower case, which removes the need to iterate the collection to find files
  • Add a normalizePath: boolean parameter to program.getFile to support skipping util.standardizePath when the caller knows the path has already been normalized

src/files/XmlFile.spec.ts Outdated Show resolved Hide resolved
src/util.ts Outdated Show resolved Hide resolved
* Given a pkgPath, remove the `pkg:/` portion of the path.
*/
public removePkgProtocol(pkgPath: string) {
if (pkgPath.startsWith('pkg:/')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add libpkg:?

Copy link
Member Author

Choose a reason for hiding this comment

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

Handled in 50a1555

pkgPath = pkgPath.replace(/\\/g, '/');
return 'pkg:/' + pkgPath;
if (!pkgPath.startsWith('pkg:/')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add libpkg:

Copy link
Member Author

Choose a reason for hiding this comment

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

Handled in 50a1555

if (targetPath === 'pkg:') {
public getPkgPathFromTarget(sourcePkgPath: string, targetPath: string) {
//handle some edge cases
if (targetPath === 'pkg:' || targetPath === 'pkg:/') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add libpkg:

Copy link
Member Author

Choose a reason for hiding this comment

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

Handled in 50a1555

@TwitchBronBron TwitchBronBron mentioned this pull request Apr 22, 2021
@TwitchBronBron
Copy link
Member Author

Closing in favor of #399

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.

2 participants