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

Fixed build script when it doesn't run on a *curses terminal and dropped hardcoded prefix for package name #47

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

blacklight
Copy link

@blacklight blacklight commented Mar 2, 2021

This PR provides:

  • A (very dirty) fix for #126. When androidjs build is run from a non-interactive terminal (e.g. a Docker build, a Makefile, an F-Droid build, or launched/piped from a non-interactive script) it won't have access to the *curses primitives, and therefore calls to methods such as clearLine and cursorTo will fail. Note however that this is a very dirty fix that simply skips those calls if the methods aren't available, but will still print out a lot of ugly all-on-one-line content that was supposed to be a progress bar. The intent is to open a discussion on what's the best approach to deal with this case - I see two options: either entirely disable the ProgressBar output if the advanced terminal drawing features are not available, or add some kind of --no-interactive option to run the script without terminal drawing options.

  • Removed the com.androidjs hardcoded prefix in the manifest generator. Hardcoding the package prefix will almost certainly conflict with the package-name specified on the package.json and result in failures when trying to upload the app to any store.

@blacklight
Copy link
Author

@Chhekur this is just a workaround to get things to work, and on a non-interactive terminal this will still print lots of progress bars but without newlines, so it's probably not the right approach. The cleanest approach would probably be to completely disable the progress bar if clearLine or cursorTo are not available.

@Chhekur
Copy link
Member

Chhekur commented Mar 4, 2021

@Chhekur this is just a workaround to get things to work, and on a non-interactive terminal this will still print lots of progress bars but without newlines, so it's probably not the right approach. The cleanest approach would probably be to completely disable the progress bar if clearLine or cursorTo are not available.

I got your point but the problem is I don't have so much time to actively work on this, right now!
if you can fix this in a better way that would be great :)

@dannickstark
Copy link

It was a great proposition. Please, can you provide a similar solution? I'm actually facing a similar problem as his description at the top.

blacklight and others added 4 commits March 14, 2022 14:24
Co-authored-by: Harendra Chhekur <820121223505e@gmail.com>
Co-authored-by: Harendra Chhekur <820121223505e@gmail.com>
Co-authored-by: Harendra Chhekur <820121223505e@gmail.com>
Co-authored-by: Harendra Chhekur <820121223505e@gmail.com>
@blacklight blacklight requested a review from Chhekur March 14, 2022 13:25
@AlexStormwood
Copy link

AlexStormwood commented Jan 27, 2024

This has been left so long!

@Chhekur please accept this PR or the other PR (#66 ), it's a blocker for using AndroidJS in CICD systems.

Issue within this repo that would be fixed: #67

Issue within the androidjs repo that could be removed when this is fixed: android-js/androidjs#161

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.

4 participants