#7 `subbash/prompt`: Any reason for adding `[` and `]` to GBranch var instead of PS1?

Closed
opened 5 years ago by tukusejssirs · 3 comments

I’ve just noticed that in the latest commit you moved the square brackets ([]) around GBranch from PS1 variable to GBranch variable itself. Any reason for this change?

This change caused more characters to be added (4 pairs instead of one).

I’ve just noticed that in the [latest commit](https://notabug.org/demure/dotfiles/commit/9fb5acac2685b4c337f76613333f6873d9aebb99) you moved the square brackets (`[]`) around `GBranch` from PS1 variable to `GBranch` variable itself. Any reason for this change? This change caused more characters to be added (4 pairs instead of one).
demure commented 5 years ago
Owner

This change was to facilitate marking a detached head state with ()'s without being surrounded by square brackets. ie [(12345678)] At the time, this seemed like the most succinct way to handle the logic as the alternative would have been to build another another Test after 'Find Branch' code section with a if/else tree of different PS1+= lines.

If you believe that the current way creates less sanitary code I can consider switching over. It was mostly an on-the-fly lines of code saving decision.

This change was to facilitate marking a detached head state with `()`'s without being surrounded by square brackets. ie `[(12345678)]` At the time, this seemed like the most succinct way to handle the logic as the alternative would have been to build another another Test after 'Find Branch' code section with a if/else tree of different `PS1+=` lines. If you believe that the current way creates less sanitary code I can consider switching over. It was mostly an on-the-fly lines of code saving decision.
tukusejssirs commented 5 years ago
Poster

Oh, I forgot about that. I think it is better to have a few more characters in comparison with additional if statement.

However, you should probably put the square brackets around ERROR, too:

                if [ -n "${GBranch}" ]; then
                    GBranch="[${GBranch}]"              ## Add brackets for final output. Will now test against brackets as well.
                    if [ "${GBranch}" == "[master]" ]; then
                        local GBranch="[M]"             ## Because why waste space
                    fi
                    ## Test if in detached head state, and set output to first 8char of hash
                    if [ "${GBranch}" == "[(detached)]" ]; then
                        GBranch="($(echo ${GStatus} | awk 'match($0,/branch.oid [0-9a-fA-F]+/) {print substr($0,RSTART+11,RLENGTH-11)}' | cut -c1-8))"
                    fi
                  else
                    local GBranch="ERROR"               ## It could happen?
                fi

PS—From English point of view, you should change It could happen? to Could it even happen?, though it is not important at all from the code point of view. :)

Oh, I forgot about that. I think it is better to have a few more characters in comparison with additional `if` statement. However, you should probably put the square brackets around `ERROR`, too: ```bash if [ -n "${GBranch}" ]; then GBranch="[${GBranch}]" ## Add brackets for final output. Will now test against brackets as well. if [ "${GBranch}" == "[master]" ]; then local GBranch="[M]" ## Because why waste space fi ## Test if in detached head state, and set output to first 8char of hash if [ "${GBranch}" == "[(detached)]" ]; then GBranch="($(echo ${GStatus} | awk 'match($0,/branch.oid [0-9a-fA-F]+/) {print substr($0,RSTART+11,RLENGTH-11)}' | cut -c1-8))" fi else local GBranch="ERROR" ## It could happen? fi ``` PS—From English point of view, you should change `It could happen?` to `Could it even happen?`, though it is not important at all from the code point of view. :)
demure commented 5 years ago
Owner

I was thinking (though I could certainly add a comment explaining this rational) that a PS1 with ERROR would help emphasize that an Issue existed and de-conflict with the chance of a branch named 'ERROR' (which would show up in the PS1 as [ERROR].

I was thinking (though I could certainly add a comment explaining this rational) that a PS1 with `ERROR` would help emphasize that an Issue existed and de-conflict with the chance of a branch named 'ERROR' (which would show up in the PS1 as `[ERROR]`.
Sign in to join this conversation.
No Milestone
No assignee
2 Participants
Loading...
Cancel
Save
There is no content yet.