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

Initial plugin architecture set up #43

Merged
merged 17 commits into from
Dec 2, 2016

Conversation

desyncr
Copy link
Member

@desyncr desyncr commented Nov 29, 2016

  • Moved most helper functions (prompt_geometry_colorize, prompt_geometry_hash_color, prompt_geometry_seconds_to_human_time, prompt_geometry_setup_async_prompt) to lib/ directory.

  • Created plugins/ directory with default plugins: git, exec_time

  • Move most variables/flags to it's plugin file

  • It does support custom prompts! Example.

  • It does works :D

TODO

  • Unregister/shutdown functions
  • Avoid registering a plugin multiple times
  • Enforce code/naming conventions
  • Update README.md
  • Review backward compatibility

Fixes #42

PROMPT_GEOMETRY_SHOW_RPROMPT=${PROMPT_GEOMETRY_SHOW_RPROMPT:-true}
PROMPT_GEOMETRY_RPROMPT_ASYNC=${PROMPT_GEOMETRY_RPROMPT_ASYNC:-true}
PROMPT_GEOMETRY_ENABLE_PLUGINS=${PROMPT_GEOMETRY_ENABLE_PLUGINS:-true}
Copy link
Member Author

Choose a reason for hiding this comment

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

This configuration is probably unnecessary, having PROMPT_GEOMETRY_SHOW_RPROMPT.

Copy link
Member

@frm frm Dec 1, 2016

Choose a reason for hiding this comment

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

What is the first variable supposed to do?

Copy link
Member Author

Choose a reason for hiding this comment

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

PROMPT_GEOMETRY_SHOW_RPROMPT is used to activate or deactive the rprompt, meanwhile PROMPT_GEOMETRY_ENABLE_PLUGINS activates the plugins (internal and custom). The issue is -currently- the rprompt only shows plugins.

@@ -0,0 +1,33 @@
# Misc configurations
GEOMETRY_ASYNC_PROMPT_TMP_FILENAME=${GEOMETRY_ASYNC_PROMPT_TMP_FILENAME:-/tmp/geometry-prompt-git-info-}
Copy link
Member Author

Choose a reason for hiding this comment

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

Change default tmp file name.


TRAPUSR1() {
# read from temp file
RPROMPT="$(<${GEOMETRY_ASYNC_PROMPT_TMP_FILENAME}$$)"
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove tmp file.

Copy link
Member

Choose a reason for hiding this comment

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

Do you have any ideas regarding an alternative for tmp files? I can see piping doing the job but is it that much necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, there is zsh-async.

geometry_plugin_setup() {
for plugin in $GEOMETRY_PROMPT_DEFAULT_PROMPTS; do
source "$GEOMETRY_ROOT/plugins/$plugin.zsh"
geometry_plugin_register $plugin
Copy link
Member Author

Choose a reason for hiding this comment

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

Probably better to leave plugin to self-register

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer having a plugin self-register.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

# Register a plugin
geometry_plugin_register() {
local plugin=$1
if [[ $plugin == "" ]]; then
Copy link
Member Author

Choose a reason for hiding this comment

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

Use argument count

local seconds=$(( total_seconds % 60 ))

if $PROMPT_GEOMETRY_GIT_TIME_SHORT_FORMAT; then
if (( days > 0 )); then
Copy link
Member Author

Choose a reason for hiding this comment

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

Move each if branch into it's own functions.

@@ -0,0 +1,5 @@
# Geometry plugins
Copy link
Member Author

Choose a reason for hiding this comment

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

  • Add example plugin structure
  • Add setup return values definition
  • Add render return values definition

Copy link
Member

Choose a reason for hiding this comment

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

I have a README almost ready, was working for the current version. Just need to adapt it. Can you change the base branch to something like 2.0 instead of master? That way I can contribute as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, I'm gonna create v2 branch and target this PR to it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. It was created from master. I guess we can already merge this feature branch and continue working on v2.

source "$GEOMETRY_ROOT/lib/plugin.zsh"
source "$GEOMETRY_ROOT/lib/time.zsh"
source "$GEOMETRY_ROOT/lib/color.zsh"
source "$GEOMETRY_ROOT/lib/grep.zsh"
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about this. Should we have a compilation script to bundle everything into geometry?

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather have that. And have it make sure we only source it once.

Copy link
Member Author

@desyncr desyncr Dec 2, 2016

Choose a reason for hiding this comment

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

Do you mean to use source instead of a makefile-like approach?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I thought you meant sourcing them all. A Makefile-like approach would have to be on install, wouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

On a build step. Maybe it's too cumbersome as of now. I'm gonna leave it as it is, sourcing them as currently implemented.

GEOMETRY_ASYNC_PROMPT_TMP_FILENAME=${GEOMETRY_ASYNC_PROMPT_TMP_FILENAME:-/tmp/geometry-prompt-git-info-}

GEOMETRY_ASYNC_PROMPT_PROC=0
prompt_geometry_setup_async_prompt() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Rename to geometry_async_setup?

Copy link
Member

Choose a reason for hiding this comment

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

👍 agreed

@@ -0,0 +1,20 @@
# Define how to colorize before the variables
prompt_geometry_colorize() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Rename to geometry_color_colorize?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure. color_colorize doesn't read well to me. Since we keep using this function throughout the prompt why not a shorthand name? Even if it's -g-color, just to save characters.

Copy link
Member Author

Choose a reason for hiding this comment

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

geometry_color_colorize would follow the format geometry_<lib>_<function>. I could also add, as an alias, a -g-color function.

Copy link
Member

Choose a reason for hiding this comment

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

Second option seems nice.

@frm
Copy link
Member

frm commented Dec 1, 2016

Really liking the way this is going. Great job!

I'd recommend, like I said in another comment, putting the PR on a different branch. That way I and someone who would like to help can jump in easily.

A lot of comments you had there could be in a TODO list, together with the list already on the PR description. I'd like to have some issues and improvements flagged for people looking for their first PR. Props for flagging some issues, really helps with that.

@desyncr desyncr changed the base branch from master to v2 December 2, 2016 01:58
local days=$(( total_seconds / 60 / 60 / 24 ))
local hours=$(( total_seconds / 60 / 60 % 24 ))
local minutes=$(( total_seconds / 60 % 60 ))
local seconds=$(( total_seconds % 60 ))

if $PROMPT_GEOMETRY_GIT_TIME_SHORT_FORMAT; then
# It looks redundant but it seems it's not
if [[ $long_format == true ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Do we need == true ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure tbh. I tried [[ $long_format ]] and it messed up. Gonna take a look later today to see what was going on.

@desyncr
Copy link
Member Author

desyncr commented Dec 2, 2016

Gonna merge to develop. I'm not squashing commits as of now to keep the changelog.

All development regarding plugins should be done in a feature branch branched off v2.

@desyncr desyncr merged commit 84a05a7 into geometry-zsh:v2 Dec 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants