Skip to content

Explore moving the command-simulating methods to their own namespace #1248

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

Closed
wants to merge 1 commit into from

Conversation

carlosmn
Copy link
Member

A lot of the methods on Repository are actually trying to replicate git
commands, which makes it harder to see what's the operation vs command.

Move them to their own namespace to make it clear what they're trying to
achieve. We start with a few forms of the git-checkout command.


This shows a possible implementation of this. The Checkout() methods on Repository would still need to get deprecated, but they depend on each other in weird ways so we can figure that out if and when we decide to go forward with it.

The library itself should also likely get a similar treatment, but it's a lot more annoying to do anything in C, so we start here.

/cc @nulltoken @jamill @whoisj

A lot of the methods on Repository are actually trying to replicate git
commands, which makes it harder to see what's the operation vs command.

Move them to their own namespace to make it clear what they're trying to
achieve. We start with a few forms of the git-checkout command.
@whoisj
Copy link

whoisj commented Dec 11, 2015

This is very nice! The only feature missing from the class is progress reporting! 😄

@carlosmn
Copy link
Member Author

Progress reporting is going to be interesting if we want to use the same output as git. We can ask the user for a Stream to treat as stdout, but how would such a stream react to CR and clearing the line for the updates in the progress output? I suppose it's not worse than GUI apps that wrap git.exe and show ^K at the end of lines.

@whoisj
Copy link

whoisj commented Dec 11, 2015

@carlosmn that is one option, another option is to return a streaming data structure which contains all of the relevant information. Should not be difficult to produce a structure which supports lines, colors, etc.

@tamlin-mike
Copy link

We can ask the user for a Stream to treat as stdout

Just a brainstorming idea, but could a callback (delegate) work for progress reporting?
Would still require all the info (f.ex. colors), though. But two ideas reaching the same conclusion "this data is required" suggests it may be on the right track (a container for progress updates - even if only a specialization of a generic ColorableMessage or whatever).

It would fit quite well into GUI's I think, while a CUI implementation would be "obvious".

@carlosmn
Copy link
Member Author

Superseded by #1294

@carlosmn carlosmn closed this Mar 30, 2016
@carlosmn carlosmn deleted the cmn/commands branch March 30, 2016 10:42
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.

3 participants