Re: [PATCH 9/9] Documentation: update api-builtin and api-setup

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Apr 5, 2011, 11:32 UTC
Message-ID: <BANLkTi=opkt7=BSCknKzN9396+urvVkhVQ@mail.gmail.com>
In-Reply-To: <20110405080729.GA25921@elie>

2011/4/5 Jonathan Nieder <jrnieder@gmail.com>:

Show 9 quoted lines

> Hi,
>
> On 2008-02-27, Nguyễn Thái Ngọc Duy wrote:
>
>>  Documentation/technical/api-builtin.txt |   10 ++++
>>  Documentation/technical/api-setup.txt   |   70 +++++++++++++++++++++++++++----
>>  2 files changed, 72 insertions(+), 8 deletions(-)
>
> I'm curious --- did anything ever come of this patch?

No idea. Perhaps nothing :) You should submit a new patch. Your description looks very good. Some comments..

Show 9 quoted lines

> Simpler advice might be:
>
>  . If the command is builtin and always needs a repository,
>   use the RUN_SETUP flag in the builtin table.
>  . If the command is builtin and can benefit from a repository,
>   use RUN_SETUP_GENTLY.
>  . If you have to run a repository search later, call
>   "setup_git_directory_gently" or the shortcut "setup_git_directory"
>   (which means "setup_git_directory_gently(NULL)").

I would discourage people from calling setup_git_directory_* directly, at least for builtins. It complicates setup handling because there will be a short time since main() until that function where we are not sure if we can access repo. There's also alias handling which may set things up while the command may not want it

I'd rather explicitly mark (RUN_SETUP_NO_THANKS?) commands that want to setup repo manually (few of them, init/clone and server commands). Then main() just goes ahead and calls setup_gently() at the very beginning. Setup hack around alias handling would be gone. If it ends up with a RUN_SETUP_NO_THANKS one, it undoes the setup. Hmm... I need to get back to this.

Show 10 quoted lines

>> +
>> +Do not access git repository (even indirectly like `git_config()`) before
>> +calling one of these functions. Otherwise you may encounter `die()` if git
>> +fails to automatically find/setup a repository.
>
>  . If you try to access the git repository (even indirectly like
>   `git_config()`) before calling setup_git_directory_gently then git
>   will look in the wrong place.
>  . When changing the value of the GIT_DIR environment variable, call
>   set_git_dir.  setup_git_directory_gently does this already.

You can also check startup_info.have_repository before you access repo because setup*gently may find no repo.

> Did we ever figure out what happens/should happen when the requested
> worktree is not an ancestor of cwd?

prefix is NULL, cwd is unchanged. Repo-wide commands should work fine. cwd-sensitive ones may cry.

Show 5 quoted lines

>> +When working with pathspecs and prefix, you can use `get_pathspec()`
>> +to auto prepend a given prefix to pathspecs. Other helpful functions
>> +are `prefix_path()`, `prefix_filename()`
>
> What do prefix_* do when there is no worktree?

These functions are purely string manipulation. If there is no worktree, prefix would be NULL and be passed to the functions as such.

-- 
Duy