Skip to content

Comments

feat: git integration (gitignore file filter and avoid overwriting uncommitted files)#1001

Merged
ATheorell merged 16 commits intoAntonOsika:mainfrom
Styren:feat_git_integration
Feb 23, 2024
Merged

feat: git integration (gitignore file filter and avoid overwriting uncommitted files)#1001
ATheorell merged 16 commits intoAntonOsika:mainfrom
Styren:feat_git_integration

Conversation

@Styren
Copy link
Collaborator

@Styren Styren commented Feb 8, 2024

Adds a basic git integration for filtering files and avoiding overwriting files accidentally, includes:

  • use gitignore to filter files from the edit list
    Initially I used fnmatch since it's builtin but unfortunately it differs too much from the git implementation to be useful. Instead it will use git check-ignore.
  • give a warning if gptengineer is about to override uncommitted changes, letting the user stage them
    It will check whether there is any overlap between the edited files and uncommitted files (whether they are tracked by git or not) and ask whether the files should be staged before being overwritten.
  • Add --yes CLI option
  • if not improve and git is installed we will initialize an empty git repo at the project root
  • Also fixed a trailing whitespace issue in ROADMAP that caused prechecks to fail

It will do a O(n^2) op when comparing file names against the gitignore. This might be unpractical if the user has a lot of ignored files, I'm not sure?

@Styren Styren changed the title feat: integrate git to filter files and verify that pending changes aren't overwritten (WIP) feat: git integration (gitignore file filter and avoid overwriting uncommitted files) Feb 16, 2024
@ATheorell
Copy link
Collaborator

@similato87 would be great to have your feedback regarding the changes to FileSelector.

@similato87
Copy link
Collaborator

@similato87 would be great to have your feedback regarding the changes to FileSelector.

The enhancement to the FileSelector part is great! Thank @Styren for implementing these changes. Introducing the ability for users to stage changes before applying them is a significant improvement. Currently, the workflow involves applying changes directly, which necessitates users to recover modified files from their local git history if needed.

My concern revolves around the possibility of incorporating a parallel functionality, similar to "restore," to ensure the "improve" function maintains a streamlined workflow. What's your idea, @ATheorell ?

@ATheorell
Copy link
Collaborator

Thanks for feeback @similato87

I believe that, there is balance between on the one hand, make sure that the users consents to changes and on the other hand, don't make too many question dialogues. Getting this balance right may be hard in one shot, so I think its fair to go for one workflow and make small changes later in case we feel like we didn't strike a good balance.

For now, I would suggest: Show the user the diffs as they are streamed (already default). Ask the user to consent to apply the changes (do you agree?). Additionally, if the user has git, we will automatically stage unstaged changes. In this case the user has the more cumbersome option of going into git and reverting manually.

What do you think @Styren ?

Copy link
Collaborator

@viborc viborc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was my mistake ( 248f6ff); I fixed it in #1021 right now and merged it.

@viborc
Copy link
Collaborator

viborc commented Feb 18, 2024

From the UX POV, I'd agree with this - makes sense.

As for the more cumbersome option I think it's an okay option - still leaves them the ability and opportunity to revert manually if something's not to their liking.

For now, I would suggest: Show the user the diffs as they are streamed (already default). Ask the user to consent to apply the changes (do you agree?). Additionally, if the user has git, we will automatically stage unstaged changes. In this case the user has the more cumbersome option of going into git and reverting manually.

@Styren
Copy link
Collaborator Author

Styren commented Feb 21, 2024

Thanks for feeback @similato87

I believe that, there is balance between on the one hand, make sure that the users consents to changes and on the other hand, don't make too many question dialogues. Getting this balance right may be hard in one shot, so I think its fair to go for one workflow and make small changes later in case we feel like we didn't strike a good balance.

For now, I would suggest: Show the user the diffs as they are streamed (already default). Ask the user to consent to apply the changes (do you agree?). Additionally, if the user has git, we will automatically stage unstaged changes. In this case the user has the more cumbersome option of going into git and reverting manually.

What do you think @Styren ?

Sounds good 👍

I've updated the PR to stage uncommitted files automatically (without consent) but ask for confirmation on improve prompts if there are any changes, irregardless of whether git is installed.

@AntonOsika
Copy link
Owner

This seems great!

What's the next step to get this merged?

@Styren
Copy link
Collaborator Author

Styren commented Feb 23, 2024

This seems great!

What's the next step to get this merged?

Fixed the last linting issues. Should be ready to merge unless you have any additional input @ATheorell ?

@ATheorell ATheorell merged commit 5e0ea19 into AntonOsika:main Feb 23, 2024
@Styren Styren deleted the feat_git_integration branch February 25, 2024 16:31
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.

5 participants