-
-
Notifications
You must be signed in to change notification settings - Fork 962
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
(feat): implement new section for haxe #1367
base: master
Are you sure you want to change the base?
(feat): implement new section for haxe #1367
Conversation
✅ Deploy Preview for spaceship-prompt ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Thanks for contributing!
Please address my comments.
sections/haxe.zsh
Outdated
|
||
|
||
SPACESHIP_HAXE_SHOW="${SPACESHIP_HAXE_SHOW=true}" | ||
SPACESHIP_HAXE_ASYNC="${SPACESHIP_HAXE_ASYNC=false}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why sync?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the idea that we are only rendering the version if the files are found and that too in the haxe project directory only,
async or sync wouldn't make much difference. Would it? Please correct me if I'm wrong...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on how fast haxe --version
works. Anyway, upsearch can also be slow to traverse the whole directory tree and check for files. It prefer to keep these runtime sections async.
@denysdovhan, I have resolved the comments, Waiting for your review. |
| `SPACESHIP_HAXE_PREFIX` | `with` | Section's prefix | | ||
| `SPACESHIP_HAXE_SUFFIX` | `$SPACESHIP_PROMPT_DEFAULT_SUFFIX` | Section's suffix | | ||
| `SPACESHIP_HAXE_SYMBOL` | `⌘` | Symbol displayed before the section | | ||
| `SPACESHIP_HAXE_COLOR` | `166` (orange) | Section's color | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add docs for SPACESHIP_HAXE_VERBOSE
option. Preferably, with an example displaying the option. Refer to other sections, like https://spaceship-prompt.sh/sections/rust/?h=#Displaying-verbose-version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but haxe --version
command doesn't yield any verbose result other than just the version number as 4.3.1
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add it in the docs, even if the output remains the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then what does this line do?
[[ $SPACESHIP_HAXE_VERBOSE == false ]] && haxe_version=${haxe_version%-*}
Should we remove it them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I guess so. Will remove in next commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, @denysdovhan, Awaiting Review.
Co-authored-by: Denys Dovhan <[email protected]>
Description
This PR adds a new section to support haxe.
Fixes #1313.
Screenshot