starcraftman / zsh-git-prompt

[Active Fork] Informative git prompt for zsh
MIT License
58 stars 15 forks source link

Work in Progess: Synchronize Haskell version to new changes #5 #19

Open madnight opened 6 years ago

madnight commented 6 years ago

This commit adds new features as described in https://github.com/starcraftman/zsh-git-prompt/issues/5

Currently i build and run it with: stack build && stack install && src/.bin/gitstatus

Example outputs: master 0 0 0 0 0 2 0 0 origin/master 0 0 :e4d4dee 0 0 6 1 0 2 0 0 .. 0 1/1

What's missing:

Let's discuss this : )

starcraftman commented 6 years ago

I look forward to all passing build. Will have to review using my rudimentary knowledge of Haskell.

Question: I noticed we have several lts resolver's named for testing in Travis. I also noted a PR in the main that has suggested bump the stack to 10.8 (latest appears to be 11.12). I assume an LTS number is a snapshot of Haskell version + packages. Should we be testing more major cuts closer to nightly? Also currently stack is only 5.0, should that bump too? I don't know how close users follow/lag. Do they come with performance improvements usually or just new features?

madnight commented 6 years ago

Your guess is right, so if i update the resolver i need to check if it still compiles and if not, i need to see what i have to change in order to use the newer lib versions.

Performance wise:

So there could be a bit of a performance improvement. Maybe we can also turn on some aggressive optimization flags, with the drawback of a increased binary size.

I'll check that out

madnight commented 6 years ago

Okay with the updated compiler flags and upgraded ghc version i am able to achieve run times as low as "cpu 0.006 total". The best run without these opts was "cpu 0.011 total".

Please note that i disabled threading, the program now only uses a single core. This is (counter-intuitively) faster than multi-threaded execution, because of the very short runtime of the program and the overhead that is introduced by starting a new cpu thread.

starcraftman commented 6 years ago

Excellent, faster always better. Thanks again for working on the patch. Once merged I'll update our main fork issue and title. Sure to be welcome news to the Haskell users and those with slower CPUs.

madnight commented 6 years ago

Should we be testing more major cuts closer to nightly?

I think, since haskell is a compiled language which allows for multiple compiler versions via stack, we can skip the matrix tests for the haskell build and do only one build with the resolver that we specify (e.g. 11.12). That is, because everyone always needs to recompile the source code on feature updates and we specify the exact set of library versions and compiler via resolver. It does not matter (stack :1st_place_medal:) which version of ghc (and haskell libs) are globally installed via the distros package maneger, therefore no need for backward or upward compatibility.

I think matrix builds with different versions only make sense if you have one global interpreter or compiler installed like (python 3.5 or node 8) and you execute the program code with that version. In this case it makes ultimately sense to support many versions since users have many different interpreter versions installed.

starcraftman commented 6 years ago

Good point regarding matrix. Quite handy having everything specified and isolated to within the project. Thankfully for python almost any recent python (except 3.1) should work.

I had a quick look at your PR and I ran into the following issue when I tried to run stack setup before building. To be clear, this old Stack command works on current master.

Downloaded lts-11.12 build plan.    
AesonException "Invalid flag name: \"bytestring--lt-0_10_4\""

I was using the packaged stack with Linux Mint 18.3, version output:

Version 0.1.10.0 x86_64

I downloaded locally the latest stack archive, and it ran fine. That stack's version is:

Version 1.7.1, Git revision 681c800873816c022739ca7ed14755e85a579565 (5807 commits) x86_64 hpack-0.28.2

I assume there is some min version of stack required to build based on current configuration, that is 1.7.1 > min > 0.1.10 . README only points to local docs on installing stack. Considering this simple event, I'm wondering if we shouldn't have a simple generic shell script like "build_haskell.sh" that does the curl/unarchive to a folder like .stack and then points absolutely to that bin in following build commands (setup/build). Makes (re)building the Haskell version streamlined, would also simplify the README instructions.

At end of script we can emit a reminder of the line required to toggle the prompt to using Haskell.

Just an idea for now, I'll continue looking at your master with 1.7.1 for now.

madnight commented 6 years ago

Yup, there is also an issue for this error: https://github.com/commercialhaskell/stackage/issues/2759#issuecomment-353373881

Yes, we could create a build shell script that fetches a recent version of stack, but i would find that a bit offensive to install another recent stack version to the users host system that might somehow interfere with stack that is installed via the package manager.

I suggest one of the following options:

Example:

FROM haskell:8.4.3
RUN mkdir -p /app
COPY . /app
WORKDIR /app
RUN stack setup && stack build
starcraftman commented 6 years ago

I took this for a spin and found a few small issues and my comments follow. Overall I'm really happy, thanks again for doing this. To test the branch I used my test_status.py to create all my test scenarios in /tmp and then simply didn't clean them up (commented out the shutil.rmtree lines). Then I changed into the scenarios directories and checked the prompt output against my python baseline.

Test Env: Stack 1.7.1, Git 2.7.1

Bug Enable ZSH with GIT_PROMPT_EXECUTABLE=haskell, cd to a non git folder and haskell prompt crashes with error. Should silently emit no text. I'm not sure if this issue predates this branch, saw similar issue on Olivier a while ago. See image below.

madnight1

Bug Rebase indicator, numbers seem reversed. Should be string '1/2' (in my test case), returns '2/1'. Simple order error, reverse them.

Bug Git repo has 1 incoming change from remote. The Haskell prompt reports 1 outgoing instead. This appears to be a simple order transposition again. Both numbers affected. To be clear, that means in the text output position 2 is number behind the remote, and position 3 is number ahead.

Additional Comments


On the topic of the more recent stack/build script, docker idea is interesting. I will look at that, I remain not super versed with docker. In general, I prefer any change that reduces user error chance to zero (even if they are programmers). That would however make us dependent then on docker, which may or may not be installed.

Alternatively, the Haskell project maintains simple fetch script (https://get.haskellstack.org/) it takes a destination flag optionally. It verifies dependencies and downloads right binary. We can use that in the script to fetch latest stack to PROJECT_ROOT/.stack, use this stack during setup/build/install (ignoring local stack), then simply delete this stack folder at end. The total download about 60 MB. The only con to this plan is uses bandwidth each time script invoked, not a big inconvenience. I think programmers would prefer not having the extra stack lying around.

madnight commented 6 years ago

Alright, i refer to your mentioned bug as bug 0,1,2 in the order you wrote them.

Bug 0 yup, somehow i thought during programming that being not in a git repository is considered to be an error, but clearly that is not the case so i corrected this. Now it is simply exit 0 without any message.

Bug 1 fixed that

Bug 2 hmm, the commits ahead and behind logic is from olivierverdier, is it a bug that is already in the current master? I can fix that no problem, but i just want to go sure.

Additional Comments 0. rebase done 1. yes i also think that tests are a very good idea, i will do some research on how to perform this kind of test with haskell

Okay i'll try to write some build script like that, if this approach works without any issues and if its only drawback is a little bandwidth usage, then it would be okay for me.

starcraftman commented 6 years ago

@madnight My bad on bug 2, apparently way back during my changes I pulled an order change on the zshrc.sh and pushed it down to gitstatus.py. You don't have to change anything, I will instead write a small patch for master that will restore order in zshrc.sh and change python output. You can then just pull it via rebase once pushed.

Regarding build script and lts resolver bump, I think we can take this to a separate issue. For this PR probably easiest to just stay on 5 for now and resolve it later. I'll copy over ideas and we can discuss/fix separately. This one already covering a lot of changes, rather it stayed focused.

madnight commented 6 years ago

Regarding build script and lts resolver bump, I think we can take this to a separate issue. For this PR probably easiest to just stay on 5 for now and resolve it later.

Okay I downgraded by dropping the upgrade commit.

madnight commented 6 years ago

I think bats https://github.com/sstephenson/bats is a nice minimal general purpose testing framework for cli applications. The good thing is that we write the test once and execute them against python and the haskell version, so no need for different tests for each language. That also eliminates the possibility of errors due to differences in the test sets between py and hs.

Example:

@test "non git dir" {
  rm -rf .git
  run gitstatus
  [ "$status" -eq 0 ]
  [ "$(gitstatus | head -c1 | wc -c)" -eq 0 ] # empty output
}

@test "1 commit" {
  add_commit "first"
  [ "$(gitstatus)" == "master 0 0 0 0 0 0 0 1 .. 0 0" ]
}

I added a squash commit with some tests, i'll write more soon.
You can run them with bats test.sh (bats required)

starcraftman commented 6 years ago

That seems like a great idea for testing. The only downside is that this kind of test framework won't integrate with the langauge and give us a line hit/miss count like currently I can get with py.test coverage. It is however nice to have a unified functional test suite that can guarantee the interface is working as expected on both though. Will also reduce some duplication and we can extract the functional tests from the unit frameworks.

I say go for it, I can always add it as a fourth environment to the tox runner and we'll just have a bit of overlap. Feel free to make liberal use of my test_gitstatus.py, I think I covered every reasonably expected repo setup.

madnight commented 6 years ago

By the way what do you think about a commit prefix like "Hask:" or something like that, so that we can differentiate between python and haskell commits in the git history?

starcraftman commented 6 years ago

@madnight That's fine by me if you'd like that. I don't think it a large deal, will be pretty clear when Haskell or other being worked on. How goes the progress on bats tests? It been a bit of a while. I been a bit busy myself last bit.

I suppose I can check out your branch and try rewriting my functional tests inside bats and PR you.

madnight commented 6 years ago

Yup, busy too here. Okay i rebased with your master again and added the prefix, i think this way it will be easier to see which commits are related to haskell in a mixed history, otherwise messages like add merge indicator would be ambiguous.

Yeah, it would be great if you could write some tests in bats : ) I guess a travis-ci integration would also be a good idea. In the meantime i figured out that many haskell projects offer a nix build. I was suprised how easy it was to build a project with nix just nix-build (and curl https://nixos.org/nix/install | sh in case the nix package manager is not installed). I'll check if this is an better option than stack, since i had some problems with stack in the past and in our case it prevents us from easy ghc upgrading.

starcraftman commented 6 years ago

@madnight Just a note, I've completed rewriting my functional tests from py.test to bats. Just wanted to let you know so no effort wasted. I've pasted the file below if you want to have a look, I'll have to make a branch from your current and do a PR onto you to merge in a bit. As I've written it we can use it to test gitstatus.py too.

Also, the Haskell version fails a small change. The "remote gone" test case. See issue #20, essentially sometimes [gone] appears instead of ahead/behind due to the remote being deleted usually.

Edit 2: PR made directly onto your fork of me.

madnight commented 6 years ago

It seems as tough the "Initial repo" tests fails in the Haskell version, but that seems also be the case for Olivier's Haskell version. This is due to git status --porcelain --branch returns ## No commits yet on master. Since this is an "old" bug, should we fix that in this MR or separate Issue?

starcraftman commented 6 years ago

Yeah, I think we've officially crammed enough into this one PR. Mark it as skip for now and we'll review that later. The remote gone test is also skipped at this moment, that was an issue raised after started.

I'll just make separate haskell issues for those two and take PRs when fixed.

madnight commented 6 years ago

Okay, I added the bats tests to travis ci (only for haskell for now) and disabled the initial repo test until the related issus is fixed.

starcraftman commented 6 years ago

Ok, I'll review this and merge unless I find anything else.

starcraftman commented 6 years ago

@madnight Sorry for the late reply, been a bit busy around the home. I have done some testing and apart from the pre-existing issues we'll resolve after I found these small issues.

1) Set Haskell prompt, then simply navigate to an empty folder. Prompt shows incorrectly 1 incoming commit. Example: [ ↓·1|✔]

When not in a git repo and STDIN passed to Haskell binary (i.e. how zshrc setup), it prints:

fatal: Not a git repository (or any parent up to mount point /home)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
 0 1 .. 0 0

When not in a git repo and no STDIN set:

(blank)

2) I notice using current master, it seems a file gets generated inside the git repository where I am currently at. File is called gitstatus.tix, seems like this should be cleaned by the haskell version itself (or simply not made). Contents: Tix [ TixModule "gitpr_Htv2saypWOm5EDTyt5uOe0/StatusParse" 4048279519 106 [0,0,8,8,8,8,8,9,32,36,36,36,8,8,8,8,8,8,8,8,8,8,8,8,8,0,0,0,8,

madnight commented 6 years ago

"Sorry for the late reply" no problem at all, I'm absolutely not in a hurry to get this merged or so, it takes as long as it takes

  1. Okay I'll check that, maybe we should also write a bats test for that.

  2. That's strange, I don't write any files in my code. Currently the only explanation that makes sense to me is that one of the library's that I included generate these .tix files for whatever reason. We could also add a bats test to check that there are no new unexpected files.

starcraftman commented 6 years ago

@madnight Any progress on these two issues?

madnight commented 6 years ago

not yet, but i have it on my TODO stack and i have holiday's soon : )

madnight commented 6 years ago

Okay Issue 2. is resolved the mysterious (.tix) is generated by Haskell Program Coverage (hpc) which I enabled via compiler flags in an earlier commit. By the way in the meantime I learned how to build Haskell projects with nix, to me it is a much simpler and cleaner approach. I added a .nix build file, which is optional, people can still build with stack if they want. It might be that you still have trouble with .tix files, that is due to stack's build cache, stack clean is not able to solve that problem and I didn't wanted to dig deeper into "yet another problem with stack". Therefore I recompiled the new build flags with the nix build ... and tada it works : ) The build cache problem is only a problem for people who compiled a version with hpc (just you and me).

Now I have a look at the first Issue.

madnight commented 6 years ago

Okay I simply exit when not in a git repo, which should fix Issue 1.

The Python script and the Haskell binary have the same output now.

git status --porcelain --branch | python gittstatus.py
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

git status --porcelain --branch | gitstatus
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
greg-murray-volusion commented 6 years ago

This looks like it's ready to merge. I'm testing it locally and haven't run into any issues. Thanks!

bruce-ricard commented 4 years ago

@starcraftman are you still trying to maintain this fork?

madnight commented 4 years ago

@bruce-ricard It doesn't seem like that. Are you particularly interested in the Haskell version?

rnc commented 4 years ago

I've since switched to https://github.com/woefe/git-prompt.zsh

bruce-ricard commented 4 years ago

Are you particularly interested in the Haskell version?

Not really no. Just interested in a maintained version.

I think I'll give a try to the one @rnc linked. Thanks!