chrisant996 / clink

Bash's powerful command line editing in cmd.exe
https://chrisant996.github.io/clink/
GNU General Public License v3.0
3.62k stars 143 forks source link

Antares Prompt Issues: Git Tag Info Missing and Long Paths Crash #701

Closed socholo closed 1 hour ago

socholo commented 3 hours ago

I really like the recent changes in Clink 1.7.0 to add prompt themes! 😀 I have been using 1.7.3 and the Antares prompt theme to get started and it works well, but I have observed 2 issues that I thought may be worth correcting (I'm happy to submit a PR, if fixing them is worthwhile):

Issue 1: Missing Git Tag Information

When a tag is checked out, the name of the tag is not appearing in the prompt. I find the tag name very useful as tells me where I am - a more significant place than just the Git hash that is dislayed.

From what I could tell, the issue is caused by the git_status coroutine and management of the associated ANTARES_GIT_HAS_TAGS and ANTARES_GIT_TAG variables: https://github.com/chrisant996/clink/blob/40e09c21d4a81ae5486b1d556a98f1d5780513c1/clink/app/themes/Antares.clinkprompt#L228-L243

The referenced variables are reset every time the prompt is refreshed - even after the referenced coroutine runs and updates those variables to contain the tag name - thus the tag information is never displayed.

I was able to correct the issue by only clearing the referenced variables when changing to:

  1. A non-Git directory
  2. A Git directory that is different from the last Git directory

Issue 2: Long Paths Cause Crash

When navigating to a directory with a very long path - one that is too long to fit the space available in the prompt - the Antares prompt will truncate the path from the left and replace with ellipses. Unfortunately, there appear to be some logic errors in the code - comparing numbers to booleans - that produces the following output:

prompt filter failed
C:\...\Antares.clinkprompt:349: attempt to compare number with boolean
stack traceback:
        C:\...\Antares.clinkprompt:465: in function 'func'
        ~clink~/app/prompt.lua:137: in function <~clink~/app/prompt.lua:124>
        [C}: in function 'xpcall'
        ~clink~/app/prompt.lua:191: in function <~clink~/app/prompt.lua:105>
        (...tail calls...)

From what I could tell, the pwd_over_half and git_over_half variables are booleans and so should not be checked if > 0, but just the value: https://github.com/chrisant996/clink/blob/40e09c21d4a81ae5486b1d556a98f1d5780513c1/clink/app/themes/Antares.clinkprompt#L349

After that issue was corrected, I noticed that the ellipsify() function has a 2 bugs:

  1. It doesn't truncate the path (it's checking an unknown variable k): https://github.com/chrisant996/clink/blob/40e09c21d4a81ae5486b1d556a98f1d5780513c1/clink/app/themes/Antares.clinkprompt#L523)
  2. It includes the final path 2x's in the result (this statement should be in the scope of truncating from the right: https://github.com/chrisant996/clink/blob/40e09c21d4a81ae5486b1d556a98f1d5780513c1/clink/app/themes/Antares.clinkprompt#L547)

If it would be worthwhile to address these issues, I'd be happy to submit a pull request with the changes that I have made to address them.

Thank you very much - I really appreciate all of the work that you have done on Clink, it's really great!

chrisant996 commented 2 hours ago

@socholo Thank you for reporting the issues, and thank you for including such thorough information!

I have a fix coming.

chrisant996 commented 2 hours ago

Re: missing tag indicator --

The coroutine shouldn't be setting self.blah variables directly. It should add a corresponding field in the table it returns, so that the caller retains control over timing (and race conditions, etc).

chrisant996 commented 1 hour ago

@socholo P.S. your timing was perfect -- I was about to publish a new update, and I was able to include this before publishing it.