git-sim icon indicating copy to clipboard operation
git-sim copied to clipboard

Error when running git-sim push/pop

Open tanmaypandey7 opened this issue 2 years ago • 3 comments
trafficstars

Hi, when trying to simulate the pop operation, i am getting the following error.

❯ git-sim stash push my-file.py
git-sim error: No modified or staged file with name: 'push'

Although, not using the push command and giving the file name directly seems to work

❯ git-sim stash my-file.py
Simulating: git stash my-file.py

Same happens with pop command.

tanmaypandey7 avatar Jan 22 '23 16:01 tanmaypandey7

Ah yes, well the reason is that so far I have only implemented the bare bones $ git-sim stash command which only accepts filename arguments to add to the stash. Currently sub-sub commands like pop and apply are not implemented, but I'll work on it and update this ticket when deployed!

initialcommit-io avatar Jan 23 '23 06:01 initialcommit-io

Hi @initialcommit-io I was working on this issue and made some progress. I was thinking to add support for push, pop, and apply subcommands for stash as others such as list, patch will be difficult to represent in the current schema of 3 columns. So git-sim stash and git-sim stash push will be effectively the same. For pop and apply I was thinking to mark data from staging to working dir. However, to differentiate between pop (which does delete the stash entry after getting POPed) and apply (which does not delete the stash entry), could have strikethrough effect on the file name for pop which indicates those changes will no longer be available in the stash and for apply we could leave it as is. Proposed apply subcommand sim:

git-sim-stash_02-05-23_21-24-10

Let me know your thoughts on it.

Also, do you know how to apply the strikethrough effect on Manim text objects? I believe we need to switch to MarkUpText but even then, I couldn't find how to apply the effect after populating the names dictionary that we have. I have 0 idea about Manim and any pointers here are appreciated!

abhijitnathwani avatar Feb 05 '23 16:02 abhijitnathwani

@abhijitnathwani Thanks a lot for this and great work on it so far. Good points about only using subcommands that make sense in a 3-column layout for now.

Also I like your idea for the strikethough for commands that delete the stashed entry. I just did some testing in Manim and was able to get the strikethrough to work like this:

m.MarkupText(
    "<span strikethrough='true' strikethrough_color='"
    + self.fontColor
    + "'>"
    + self.trim_path(f)
    + "</span>",
    font="Monospace",
    font_size=24,
    color=self.fontColor,
    )

The text objects are currently created in the setup_and_draw_zones() method in GitSimBaseCommand. I see 2 options for integrating this:

  1. Add a keyword argument like strikethrough=False to that method, where you can pass in one or more column indexes to strike through.

  2. Separate the text object creation starting on line 617 into a new method like "createZoneText()" that can be overridden in command subclasses like GitSimStash to implement the strikethrough there.

I'll leave that decision up to you, if you think one option works better than the other.

initialcommit-io avatar Feb 05 '23 19:02 initialcommit-io

@abhijitnathwani FYI I worked with @paketb0te to do some pretty significant refactoring to switch from argparse to Typer. Just wanted to give you a heads up since I know you already started on this.

initialcommit-io avatar Feb 10 '23 00:02 initialcommit-io

@initialcommit-io yeah, I saw the PR getting merged into the main branch. I was following the discussion on the Typer. I thought I'd finish this PR before that :D Anyway, I'll go through the required changes for parsing the arguments using Typer and implement accordingly. Thanks for the heads up!

abhijitnathwani avatar Feb 10 '23 03:02 abhijitnathwani

@abhijitnathwani Thanks for this and for accommodating the big changes...

initialcommit-io avatar Feb 11 '23 19:02 initialcommit-io

Hi @initialcommit-io , Sorry for the delay in completing this. I am done with the feature development. I had one quick question for you. I am not sure how do we want to handle the files list in stash pop or stash apply ? Generally, files are not a part of the git stash subcommand as it accepts the stash entry to be applied. I was thinking to add a print if the user adds files as the argument to apply/pop and simulate it on all the changed files in the working directory.

Proposed approach:

PS D:\git-sim> git-sim stash pop git_sim/stash.py
Files are not required in apply/pop subcommand. Ignoring the file list.....
Simulating: git stash pop git_sim/stash.py
Output image location: D:\foss_contribution\git-sim\git-sim_media\images\git-sim-stash_02-15-23_21-42-48.jpg
PS D:\git-sim> git-sim stash pop
Simulating: git stash pop 
Output image location: D:\foss_contribution\git-sim\git-sim_media\images\git-sim-stash_02-15-23_21-43-01.jpg

abhijitnathwani avatar Feb 15 '23 16:02 abhijitnathwani

@abhijitnathwani I am not sure if you saw it in the discussion of #43, HERE are the docs for nested subcommands in Typer :)

paketb0te avatar Feb 15 '23 17:02 paketb0te

@abhijitnathwani Sure I think that sounds good. One thing to note, I added a --stdout option that writes the image data directly to standard output, so in the case that is set we need to suppress all printed output so it doesn't interfere with the raw image data.

Feel free to submit the PR when ready and I'll take a look.

initialcommit-io avatar Feb 15 '23 17:02 initialcommit-io