Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Jan 16, 2014

I found the "gitx" command line utility was not correctly handling arguments and tried to fix it.
I believe $ gitx (one or more branch names or commit SHAs) should work on the command line now.

When the CLI command is invoked without any arguments or with some refs,
the CLI command was using a low-level NSAppleEventDescriptor to
communicate with the main GUI process. That behavior was inconsistent
with the behavior for other command-line options, where communication is
done with high-level Scripting Bridge. Moreover, the GUI side was
failing to handle the NSAppleEventDescriptor and thus ignoring command
line arguments passed from CLI (cf. #196).

This commit modifies the code so that CLI and GUI always use Scripting
Bridge to send and receive command-line arguments correctly.
Recreating the graph queue means that the new queue may start a
background operation before the old queue is still running another
operation. Multiple operations being executed at the same time seem to
be causing an occasional crash because they are not synchronized.

Note that calling cancelAllOperations for the old queue does not abort
an operation that has already started.
The rev specifier strings may not always simple symbolic refs. They may
be SHA values or refs with some modifiers like HEAD~3^2. They needs to
be rev-parsed to load commits as expected.
Refs that are not branches, tags, nor remotes should be added to the
"others" list rather than the "branches" list.
If the ref is a SHA string rather than an ordinary named ref, the
"referenceByLookingUpReferencedNamed:..." method doesn't find the
expected GTReference. The "lookupObjectByRevParse:..." method can be
used to parse SHA strings as well as named refs.
@rowanj
Copy link
Owner

rowanj commented Jan 22, 2014

Sounds good, will review.

@tiennou
Copy link

tiennou commented Mar 2, 2014

I'm working on removing the dependency gitx has on git and Objective-Git, and cleaning up how it handles its arguments. I have a tendency to expect gitx *something* to open a new repository for that folder (usually gitx submodule-path or gitx ..), which it doesn't currently handle without a --git-dir argument.

So that conflicts with what d2c426c does here, as well as not being too clean IMHO (this passes arguments as raw strings through AppleScript, while it would be better to have real options for that).

@aYukiWatanabe What's the actual use case you're having in mind ? I can't understand what you're expecting gitx (one or more branch names or commit SHAs) to do (especially w.r.t "one or more"). gitx already has a --branch flag to select one branch by name in the repository you're currently in (or the one you're passing to --git-dir), and maybe an --sha option would be better than overloading what an "untagged" argument means ?

@ghost
Copy link
Author

ghost commented Mar 3, 2014

Before gitx switched to Objective-Git, gitx used to accept any arguments that are accepted by git log. I really liked that behavior, since I had already been accustomed with the syntax of git log. Basically I tried to restore the old behavior in this pull request, though I couldn't manage to restore it perfectly.

I often compare the history between the master branch and a working branch by git log --graph --oneline master my_branch and now I'm expecting gitx to show a similar graph when I run gitx master my_branch.
Viewing all (local) branches is sometimes not very useful for me because the graph includes other unrelated branches, so I want a way to show only selected branches.

By the way, I'm not sure if gitx should fully restore the old argument syntax support. AFAIK, Objective-Git and libgit don't seem to have API to parse arguments in the same way as git log. It would be hard, if not impossible, to make gitx imitate git log's argument parsing behavior. It could be a good option to abandon the git log syntax support and introduce gitx's own options.

@tiennou tiennou mentioned this pull request Mar 29, 2014
@ghost ghost closed this Jun 19, 2015
This pull request was closed.
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