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

Add an item for jj #530

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add an item for jj #530

wants to merge 1 commit into from

Conversation

ETCaton
Copy link

@ETCaton ETCaton commented Sep 14, 2024

Description

Adds an item for jj based on their fish and Starship prompts.

Screenshots (if appropriate)

d7499e4150e8e51951f007f88214cad51bf23d4631dd923ea7e73f6682fa7077

As shown in those prompt examples, you can get diff info etc. out of it. Unfortunately it relies on sd (I think?) and jj currently outputs a non-formattable output for diffs.

Rather than doing custom parsing, I just made the ultimate output a joined array so users can fiddle with adding more info relatively easily. For example I added the (slightly modified) jjstate from Starship's prompt as a separate variable, then appended to the array.

a44ac2a43c6ab9dd7b58b6976e5855a3bc362a86489a5c8232e5e0c8b0c94d70

How Has This Been Tested

  • I have tested using Linux.
  • I have tested using MacOS.

Checklist

  • I am ready to update the wiki accordingly.
  • I have updated the tests accordingly.

@bbigras
Copy link

bbigras commented Sep 29, 2024

It seems to be working pretty well for me:

image

Thank you very much!

Btw, the image is from a colocated repo. I wonder if it would be useful (and doable) to hide the normal git item from the prompt when the jj item is active.

@ETCaton
Copy link
Author

ETCaton commented Sep 29, 2024

I could see an argument for that, but jj is also niche and early days. If they were at 1.0/feature parity with things like submodules, etc. I might be more inclined to attempt excluding Git if jj appears to be configured for the repo as well.

For now my opinion is that people can just customize their prompt to exclude Git if they like. I don’t think Starship even allows disabling the Git repo status, so the existence of that jj prompt for it probably means that’s acceptable enough for the time being.

@bbigras
Copy link

bbigras commented Sep 29, 2024

For now my opinion is that people can just customize their prompt to exclude Git if they like.

Note that I still use git for some projects.

But yeah, it's fine like this for now.

Thanks again.

@bbigras
Copy link

bbigras commented Oct 5, 2024

❯ Warning: In template expression
 --> 3:9
  |
3 |         branches.map(|x| if(
  |         ^------^
  |
  = branches() is deprecated; use bookmarks() instead

https://github.com/martinvonz/jj/releases/tag/v0.22.0

@ETCaton
Copy link
Author

ETCaton commented Oct 5, 2024

Thanks for noticing that! I'm using the version from martinvonz/jj#3191 so I didn't get that warning (yet)

@bbigras
Copy link

bbigras commented Oct 9, 2024

It works, thanks!

@bbigras
Copy link

bbigras commented Oct 23, 2024

Any progress on getting this merged? It works great for me.

@thor314
Copy link

thor314 commented Oct 26, 2024

I wanted to try out your jj prompt @ETCaton, but after uninstalling Tide and re-installing from a local clone of your repo, I'm still seeing the normal git prompt. Am I missing something?

I ran:

fisher remove IlanCosman/tide
git clone [email protected]:ETCaton/tide.git && cd tide
git checkout jj-item
fisher install . 
# <run configuration script>
# open a new terminal and cd to a colocated jj dir, don't see jj info

@ETCaton
Copy link
Author

ETCaton commented Oct 26, 2024

Any progress on getting this merged? It works great for me.

Looking at repo and author activity, it seems like they’re currently a little busy. I don’t see a particularly active fork to imply people are moving away from this, or merging in patches themselves in the meantime, so for now I think copying this file would suffice. Only changes I intend to make to this PR would be ones to accommodate jj changes upstream like the branch->bookmark renaming

I'm still seeing the normal git prompt

This doesn’t suppress the Git prompt, so that’s expected. If you mean you’re not seeing jj I’m not sure; have you customized your prompt/does explicitly adding this make it show up?

@thor314
Copy link

thor314 commented Oct 26, 2024

@ETCaton I'm unsure what "explicitly adding this make it show up means".

on a terminal with tide removed:


thor@crow ~/tide ((no description set) n 45 (empty))>

I've set the can see the jj descriptors via the script borrowd from this gist: https://gist.github.com/hroi/d0dc0e95221af858ee129fd66251897e/

after fisher install . from your tide fork:


~/tide jj-item 
❯

that is, no jj info.

On running the function you added, issues elsewhere in tide are reported:

❯ _tide_item_jj
test: Missing argument at index 3
= left
       ^
~/.config/fish/functions/_tide_print_item.fish (line 9):
    else if test $_tide_side = left
            ^
in function '_tide_print_item' with arguments 'jj \(\e\[1m\e\[38\;5
\;5mq\e\[0m\ \e\[1m\e\[38\;5\;4mc\e\[0m\)'
        called on line 32 of file ~/.config/fish/functions/_tide_it
em_jj.fish
in function '_tide_item_jj'
 (q c)⏎

~/.files/fish @12a9aa2 !2 
❯

@dragonmaus
Copy link

@thor314

Re: "explicitly adding this"
Inserting jj into the tide_left_prompt_items universal variable seems to be sufficient.

I also inserted .jj into the tide_pwd_markers universal variable, for good measure.

@thor314
Copy link

thor314 commented Oct 26, 2024

thanks dragon. I asked claude how to do what you told me to, and after running:

set -U tide_left_prompt_items $tide_left_prompt_items jj

# Add .jj to the pwd markers
set -U tide_pwd_markers $tide_pwd_markers .jj

I see in my prompt:
~/.files @be7533b ❯ (w 2)

this is a little out of order, so I adjusted things around, and now we've got the right order. Cheers, love the tool.

❯ echo $tide_left_prompt_items
pwd git newline character
❯ set -U tide_left_prompt_items pwd git jj newline character

additionally, if relevant, the initial PR was marked tested on OS X. I have tested on Linux.

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