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

Error when running git-sim stash push/pop #1

Closed
tanmaypandey7 opened this issue Jan 22, 2023 · 9 comments · Fixed by #60
Closed

Error when running git-sim stash push/pop #1

tanmaypandey7 opened this issue Jan 22, 2023 · 9 comments · Fixed by #60
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@tanmaypandey7
Copy link

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.

@initialcommit-io
Copy link
Contributor

initialcommit-io commented Jan 23, 2023

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 initialcommit-io added enhancement New feature or request good first issue Good for newcomers labels Feb 2, 2023
@abhijitnathwani
Copy link
Contributor

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!

@initialcommit-io
Copy link
Contributor

@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
Copy link
Contributor

@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.

@abhijitnathwani
Copy link
Contributor

@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!

@initialcommit-io
Copy link
Contributor

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

@abhijitnathwani
Copy link
Contributor

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

@paketb0te
Copy link
Contributor

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

@initialcommit-io
Copy link
Contributor

@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 initialcommit-io changed the title Error when running git-sim push/pop Error when running git-sim stash push/pop Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants