Git development
 help / color / mirror / Atom feed
* [PATCHv5 2/6] gitweb: Add manpage for gitweb configuration files
From: Jakub Narebski @ 2011-10-08 18:31 UTC (permalink / raw)
  To: git, Drew Northup; +Cc: Jonathan Nieder, Jakub Narebski
In-Reply-To: <1318098723-12813-1-git-send-email-jnareb@gmail.com>

From: Drew Northup <drew.northup@maine.edu>

Much of what is in gitweb.conf.txt has been pulled directly from the
README file of gitweb.  The manpage was supplemented with description
of missing gitweb config variables, and with description of gitweb's
%features.

There remains a bit of redundancy, which should be reduced if
possible... but I think some of duplication of information is
inevitable.

[jn: Improved, extended, removed duplicate info from README]

Signed-off-by: Drew Northup <drew.northup@maine.edu>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
---
I refer to my reply to Jonathan Nieder in

  Re: [PATCH/RFCv4 2/4] gitweb: Add manpage for /etc/gitweb.conf (for gitweb documentation)
  http://thread.gmane.org/gmane.comp.version-control.git/181605/focus=181896

for description of changes (and to parent post).

Thanks a lot to Jonathan Nieder.

 Documentation/Makefile        |    2 +-
 Documentation/gitweb.conf.txt |  866 +++++++++++++++++++++++++++++++++++++++++
 gitweb/README                 |  133 +-------
 3 files changed, 868 insertions(+), 133 deletions(-)
 create mode 100644 Documentation/gitweb.conf.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 44be67b..6d71943 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -3,7 +3,7 @@ MAN1_TXT= \
 		$(wildcard git-*.txt)) \
 	gitk.txt git.txt
 MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt githooks.txt \
-	gitrepository-layout.txt
+	gitrepository-layout.txt gitweb.conf.txt
 MAN7_TXT=gitcli.txt gittutorial.txt gittutorial-2.txt \
 	gitcvs-migration.txt gitcore-tutorial.txt gitglossary.txt \
 	gitdiffcore.txt gitnamespaces.txt gitrevisions.txt gitworkflows.txt
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
new file mode 100644
index 0000000..31eb527
--- /dev/null
+++ b/Documentation/gitweb.conf.txt
@@ -0,0 +1,866 @@
+gitweb.conf(5)
+==============
+
+NAME
+----
+gitweb.conf - Gitweb (git web interface) configuration file
+
+SYNOPSIS
+--------
+/etc/gitweb.conf, /etc/gitweb-common.conf, $GITWEBDIR/gitweb_config.perl
+
+DESCRIPTION
+-----------
+
+The gitweb CGI script for viewing Git repositories over the web uses a
+perl script fragment as its configuration file.  You can set variables
+using "`our $variable = value`"; text from a "#" character until the
+end of a line is ignored.  See *perlsyn*(1) for details.
+
+An example:
+
+    # gitweb configuration file for http://git.example.org
+    #
+    our $projectroot = "/srv/git"; # FHS recommendation
+    our $site_name = 'Example.org >> Repos';
+
+
+The configuration file is used to override the default settings that
+were built into gitweb at the time the 'gitweb.cgi' script was generated.
+
+While one could just alter the configuration settings in the gitweb
+CGI itself, those changes would be lost upon upgrade.  Configuration
+settings might also be placed into a file in the same directory as the
+CGI script with the default name 'gitweb_config.perl' -- allowing
+one to have multiple gitweb instances with different configurations by
+the use of symlinks.
+
+
+DISCUSSION
+----------
+Gitweb reads configuration data from the following sources in the
+following order:
+
+ * built-in values (some set during build stage),
+
+ * common system-wide configuration file (defaults to
+   '/etc/gitweb-common.conf'),
+
+ * either per-instance configuration file (defaults to 'gitweb_config.perl'
+   in the same directory as the installed gitweb), or if it does not exists
+   then fallback system-wide configuration file (defaults to '/etc/gitweb.conf').
+
+Values obtained in later configuration files override values obtained earlier
+in the above sequence.
+
+Locations of the common system-wide configuration file, the fallback
+system-wide configuration file and the per-instance configuration file
+are defined at compile time using build-time Makefile configuration
+variables, respectively `GITWEB_CONFIG_COMMON`, `GITWEB_CONFIG_SYSTEM`
+and `GITWEB_CONFIG`.
+
+You can also override locations of gitweb configuration files during
+runtime by setting the following environment variables:
+`GITWEB_CONFIG_COMMON`, `GITWEB_CONFIG_SYSTEM` and `GITWEB_CONFIG`
+to a non-empty value.
+
+
+The syntax of the configuration files is that of Perl, since these files are
+handled by sourcing them as fragments of Perl code (the language that
+gitweb itself is written in). Variables are typically set using the
+`our` qualifier (as in "`our $variable = <value>;`") to avoid syntax
+errors if a new version of gitweb no longer uses a variable and therefore
+stops declaring it.
+
+You can include other configuration file using read_config_file()
+subroutine.  For example, one might want to put gitweb configuration
+related to access control for viewing repositories via Gitolite (one
+of git repository management tools) in a separate file, e.g. in
+'/etc/gitweb-gitolite.conf'.  To include it, put
+
+--------------------------------------------------
+read_config_file("/etc/gitweb-gitolite.conf");
+--------------------------------------------------
+
+somewhere in gitweb configuration file used, e.g. in per-installation
+gitweb configuration file.  Note that read_config_file() checks itself
+that the file it reads exists, and does nothing if it is not found.
+It also handles errors in included file.
+
+
+The default configuration with no configuration file at all may work
+perfectly well for some installations.  Still, a configuration file is
+useful for customizing or tweaking the behavior of gitweb in many ways, and
+some optional features will not be present unless explicitly enabled using
+the configurable `%features` variable (see also "Configuring gitweb
+features" section below).
+
+
+CONFIGURATION VARIABLES
+-----------------------
+Some configuration variables have their default values (embedded in the CGI
+script) set during building gitweb -- if that is the case, this fact is put
+in their description.  See gitweb's 'INSTALL' file for instructions on building
+and installing gitweb.
+
+
+Location of repositories
+~~~~~~~~~~~~~~~~~~~~~~~~
+The configuration variables described below control how gitweb finds
+git repositories, and how repositories are displayed and accessed.
+
+$projectroot::
+	Absolute filesystem path which will be prepended to project path;
+	the path to repository is `$projectroot/$project`.  Set to
+	`$GITWEB_PROJECTROOT` during installation.  This variable has to be
+	set correctly for gitweb to find repositories.
++
+For example, if `$projectroot` is set to "/srv/git" by putting the following
+in gitweb config file:
++
+----------------------------------------------------------------------------
+our $projectroot = "/srv/git";
+----------------------------------------------------------------------------
++
+then
++
+------------------------------------------------
+http://git.example.com/gitweb.cgi?p=foo/bar.git
+------------------------------------------------
++
+and its path_info based equivalent
++
+------------------------------------------------
+http://git.example.com/gitweb.cgi/foo/bar.git
+------------------------------------------------
++
+will map to the path '/srv/git/foo/bar.git' on the filesystem.
+
+$projects_list::
+	Name of a plain text file listing projects, or a name of directory
+	to be scanned for projects.
++
+Project list files should list one project per line, with each line
+having the following format
++
+-----------------------------------------------------------------------------
+<URI-encoded filesystem path to repository> SP <URI-encoded repository owner>
+-----------------------------------------------------------------------------
++
+The default value of this variable is determined by the `GITWEB_LIST`
+makefile variable at installation time.  If this variable is empty, gitweb
+will fall back to scanning the `$projectroot` directory for repositories.
+
+$project_maxdepth::
+	If `$projects_list` variable is unset, gitweb will recursively
+	scan filesystem for git repositories.  The `$project_maxdepth`
+	is used to limit traversing depth, relative to `$projectroot`
+	(starting point); it means that directories which are further
+	from `$projectroot` than `$project_maxdepth` will be skipped.
++
+It is purely performance optimization, originally intended for MacOS X,
+where recursive directory traversal is slow.  Gitweb follows symbolic
+links, but it detects cycles, ignoring any duplicate files and directories.
++
+The default value of this variable is determined by the build-time
+configuration variable `GITWEB_PROJECT_MAXDEPTH`, which defaults to
+2007.
+
+$export_ok::
+	Show repository only if this file exists (in repository).  Only
+	effective if this variable evaluates to true.  Can be set when
+	building gitweb by setting `GITWEB_EXPORT_OK`.  This path is
+	relative to `GIT_DIR`.  git-daemon[1] uses 'git-daemon-export-ok',
+	unless started with `--export-all`.  [No default / Not set
+	(this feature is turned off)]
+
+$export_auth_hook::
+	Function used to determine which repositories should be shown.
+	This subroutine should take one parameter, the full path to
+	a project, and if it returns true, that project will be included
+	in the projects list and can be accessed through gitweb as long
+	as it fulfills the other requirements described by $export_ok,
+	$projects_list, and $projects_maxdepth.  Example:
++
+----------------------------------------------------------------------------
+our $export_auth_hook = sub { return -e "$_[0]/git-daemon-export-ok"; };
+----------------------------------------------------------------------------
++
+though the above might be done by using `$export_ok` instead
++
+----------------------------------------------------------------------------
+our $export_ok = "git-daemon-export-ok";
+----------------------------------------------------------------------------
++
+If not set (default), it means that this feature is disabled.
+
+$strict_export::
+	Only allow viewing of repositories also shown on the overview page.
+	This for example makes `$gitweb_export_ok` file decide if repository is
+	available and not only if it is shown.  If `$gitweb_list` points to
+	file with list of project, only those repositories listed would be
+	available for gitweb.  Can be set during building gitweb via
+	`GITWEB_STRICT_EXPORT`.  [No default / Not set (you can access
+	repositories hidden from projects list page)]
+
+
+Finding files
+~~~~~~~~~~~~~
+The following configuration variables tell gitweb where to find files.
+The values of these variables are paths on the filesystem.
+
+$GIT::
+	Core git executable to use.  By default set to `$GIT_BINDIR/git`, which
+	in turn is by default set to `$(bindir)/git`.  If you use git installed
+	from a binary package, you should usually set this to "/usr/bin/git".
+	This can just be "git" if your web server has a sensible PATH; from
+	security point of view it is better to use absolute path to git binary.
+	If you have multiple git versions installed it can be used to choose
+	which one to use.  Must be (correctly) set for gitweb to be able to
+	work.
+
+$mimetypes_file::
+	File to use for (filename extension based) guessing of MIME types before
+	trying '/etc/mime.types'.  *NOTE* that this path, if relative, is taken
+	as relative to the current git repository, not to CGI script.  If unset,
+	only '/etc/mime.types' is used (if present on filesystem).  If no mimetypes
+	file is found, mimetype guessing based on extension of file is disabled.
+	[Unset by default]
+
+$highlight_bin::
+	Path to the highlight executable to use (it must be the one from
+	http://www.andre-simon.de due to assumptions about parameters and output).
+	Useful if 'highlight' is not installed on your web server's PATH.
+	[Default: 'highlight']
++
+*NOTE*: if you want to add support for new file type (supported by
+"highlight" but not used by gitweb), you need to modify `%highlight_ext`
+or `%highlight_basename`, depending on whether you detect type of file
+based on extension (for example "sh") or on its basename (for example
+"Makefile").  The keys of these hashes are extension and basename,
+respectively, and value for given key is name of syntax to be passed via
+`--syntax <syntax>` to highlighter.
++
+For example if repositories you are hosting use "phtml" extension for
+PHP files, and you want to have correct syntax-highlighting for those
+files, you can add the following to gitweb configuration:
++
+---------------------------------------------------------
+our %highlight_ext;
+$highlight_ext{'phtml'} = 'php';
+---------------------------------------------------------
+
+
+Links and their targets
+~~~~~~~~~~~~~~~~~~~~~~~
+The configuration variables described below configure some of gitweb links:
+their target and their look (text or image), and where to find page
+prerequisites (stylesheet, favicon, images, scripts).  Usually they are left
+at their default values, with the possible exception of `@stylesheets`
+variable.
+
+@stylesheets::
+	List of URIs of stylesheets (relative to the base URI of a page). You
+	might specify more than one stylesheet, for example to use "gitweb.css"
+	as base with site specific modifications in a separate stylesheet
+	to make it easier to upgrade gitweb.  For example, you can add
+	a `site` stylesheet by putting
++
+----------------------------------------------------------------------------
+push @stylesheets, "gitweb-site.css";
+----------------------------------------------------------------------------
++
+in the gitweb config file.  Those values that are relative paths are
+relative to base URI of gitweb.
++
+This list should contain the URI of gitweb's standard stylesheet.  The default
+URI of gitweb stylesheet can be set at build time using the `GITWEB_CSS`
+makefile variable.  Its default value is 'static/gitweb.css'
+(or 'static/gitweb.min.css' if the `CSSMIN` variable is defined,
+i.e. if CSS minifier is used during build).
++
+*Note*: there is also a legacy `$stylesheet` configuration variable, which was
+used by older gitweb.  If `$stylesheet` variable is defined, only CSS stylesheet
+given by this variable is used by gitweb.
+
+$logo::
+	Points to the location where you put 'git-logo.png' on your web
+	server, or to be more the generic URI of logo, 72x27 size).  This image
+	is displayed in the top right corner of each gitweb page and used as
+	a logo for the Atom feed.  Relative to the base URI of gitweb (as a path).
+	Can be adjusted when building gitweb using `GITWEB_LOGO` variable
+	[Default: 'static/git-logo.png']
+
+$favicon::
+	Points to the location where you put 'git-favicon.png' on your web
+	server, or to be more the generic URI of favicon, which will be served
+	as "image/png" type.  Web browsers that support favicons (website icons)
+	may display them in the browser's URL bar and next to the site name in
+	bookmarks.  Relative to the base URI of gitweb.  Can be adjusted at
+	build time using `GITWEB_FAVICON` variable.
+	[Default: 'static/git-favicon.png']
+
+$javascript::
+	Points to the location where you put 'gitweb.js' on your web server,
+	or to be more generic the URI of JavaScript code used by gitweb.
+	Relative to the base URI of gitweb.  Can be set at build time using
+	the `GITWEB_JS` build-time configuration variable.
++
+The default value is either 'static/gitweb.js', or 'static/gitweb.min.js' if
+the `JSMIN` build variable was defined, i.e. if JavaScript minifier was used
+at build time.  *Note* that this single file is generated from multiple
+individual JavaScript "modules".
+
+$home_link::
+	Target of the home link on the top of all pages (the first part of view
+	"breadcrumbs").  By default it is set to the absolute URI of a current page
+	(to the value of `$my_uri` variable, or to "/" if `$my_uri` is undefined
+	or is an empty string).
+
+$home_link_str::
+	Label for the "home link" at the top of all pages, leading to `$home_link`
+	(usually the main gitweb page, which contains the projects list).  It is
+	used as the first component of gitweb's "breadcrumb trail":
+	`<home link> / <project> / <action>`.  Can be set at build time using
+	the `GITWEB_HOME_LINK_STR` variable.  [Default: "projects"]
+
+$logo_url::
+$logo_label::
+	URI and label (title) for the Git logo link (or your site logo,
+	if you chose to use different logo image). By default, these both
+	refer to git homepage; in the past, they pointed to git documentation
+	at http://www.kernel.org[].
+
+
+Changing gitweb's look
+~~~~~~~~~~~~~~~~~~~~~~
+You can adjust how pages generated by gitweb look using the variables described
+below.  You can change the site name, add common headers and footers for all
+pages, and add a description of this gitweb installation on its main page
+(which is the projects list page), etc.
+
+$site_name::
+	Name of your site or organization, to appear in page titles.  Set it
+	to something descriptive for clearer bookmarks etc.  If this variable
+	is not set or is, then gitweb uses the value of the `SERVER_NAME`
+	CGI environment variable, setting site name to "$SERVER_NAME Git",
+	or "Untitled Git" if this variable is not set (e.g. if running gitweb
+	as standalone script).
++
+Can be set using the `GITWEB_SITENAME` at build time.  [No default]
+
+$site_header::
+	Name of a file with HTML to be included at the top of each page.
+	Relative to the directory containing the 'gitweb.cgi' script.
+	Can be set using `GITWEB_SITE_HEADER` at build time.  [No default]
+
+$site_footer::
+	Name of a file with HTML to be included at the bottom of each page.
+	Relative to the directory containing the 'gitweb.cgi' script.
+	Can be set using `GITWEB_SITE_FOOTER` at build time.  [No default]
+
+$home_text::
+	Name of a HTML file which, if it exists, is included on the
+	gitweb projects overview page ("projects_list" view).  Relative to
+	the directory containing the gitweb.cgi script.  Default value
+	can be adjusted during build time using `GITWEB_HOMETEXT` variable.
+	[Default: 'indextext.html']
+
+$projects_list_description_width::
+	The width (in characters) of the "Description" column of the projects list.
+	Longer descriptions will be truncated (trying to cut at word boundary);
+	the full description is available in the 'title' attribute (usually shown on
+	mouseover).  The default is 25, which might be too small if you
+	use long project descriptions.
+
+$default_projects_order::
+	Default value of ordering of projects on projects list page, which
+	means the ordering used if you don't explicitly sort projects list
+	(if there is no "o" CGI query parameter in the URL).  Valid values
+	are "none" (unsorted), "project" (projects are by project name,
+	i.e. path to repository relative to `$projectroot`), "descr"
+	(project description), "owner", and "age" (by date of most current
+	commit).
++
+Default value is "project".  Unknown value means unsorted.
+
+
+Changing gitweb's behavior
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+These configuration variables control _internal_ gitweb behavior.
+
+$default_blob_plain_mimetype::
+	Default mimetype for the blob_plain (raw) view, if mimetype checking
+	doesn't result in some other type; by default "text/plain".
+	Gitweb guesses mimetype of a file to display based on extension
+	of its filename, using `$mimetypes_file` (if set and file exists)
+	and '/etc/mime.types' files (see *mime.types*(5) manpage; only
+	filename extension rules are supported by gitweb).
+
+$default_text_plain_charset::
+	Default charset for text files. If this is not set, the web server
+	configuration will be used.  Unset by default.
+
+$fallback_encoding::
+	Gitweb assumes this charset when a line contains non-UTF-8 characters.
+	The fallback decoding is used without error checking, so it can be even
+	"utf-8". The value must be a valid encoding; see the *Encoding::Supported*(3pm)
+	man page for a list. The default is "latin1", aka. "iso-8859-1".
+
+@diff_opts::
+	Rename detection options for git-diff and git-diff-tree. The default is
+	(\'-M'); set it to (\'-C') or (\'-C', \'-C') to also detect copies,
+	or set it to () i.e. empty list if you don't want to have renames
+	detection.
++
+*Note* that rename and especially copy detection can be quite
+CPU-intensive.  Note also that non git tools can have problems with
+patches generated with options mentioned above, especially when they
+involve file copies (\'-C') or criss-cross renames (\'-B').
+
+
+Some optional features and policies
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Most of features are configured via `%feature` hash; however some of extra
+gitweb features can be turned on and configured using variables described
+below.  This list beside configuration variables that control how gitweb
+looks does contain variables configuring administrative side of gitweb
+(e.g. cross-site scripting prevention; admittedly this as side effect
+affects how "summary" pages look like, or load limiting).
+
+@git_base_url_list::
+	List of git base URLs.  These URLs are used to generate URLs
+	describing from where to fetch a project, which are shown on
+	project summary page.  The full fetch URL is "`$git_base_url/$project`",
+	for each element of this list. You can set up multiple base URLs
+	(for example one for `git://` protocol, and one for `http://`
+	protocol).
++
+Note that per repository configuration can be set in '$GIT_DIR/cloneurl'
+file, or as values of multi-value `gitweb.url` configuration variable in
+project config.  Per-repository configuration takes precedence over value
+composed from `@git_base_url_list` elements and project name.
++
+You can setup one single value (single entry/item in this list) at build
+time by setting the `GITWEB_BASE_URL` built-time configuration variable.
+[Default: (), i.e. empty list]
+
+$projects_list_group_categories::
+	Whether to enables the grouping of projects by category on the project
+	list page. The category of a project is determined by the
+	`$GIT_DIR/category` file or the `gitweb.category` variable in each
+	repository's configuration.  [Disabled by default].
+
+$project_list_default_category::
+	Default category for projects for which none is specified.  If this is
+	set to the empty string, such projects will remain uncategorized and
+	listed at the top, above categorized projects.  Used only if project
+	categories are enabled, which means if `$projects_list_group_categories`
+	is true.  [Default: "" (empty string)]
+
+$prevent_xss::
+	If true, some gitweb features are disabled to prevent content in
+	repositories from launching cross-site scripting (XSS) attacks.  Set this
+	to true if you don't trust the content of your repositories.
+	[Default: false].
+
+$maxload::
+	Used to set the maximum load that we will still respond to gitweb queries.
+	If the server load exceeds this value then gitweb will return
+	"503 Service Unavailable" error.  The server load is taken to be 0
+	if gitweb cannot determine its value.  Currently it works only on Linux,
+	where it uses '/proc/loadavg'; the load there is the number of active
+	tasks on the system -- processes that are actually running -- averaged
+	over the last minute.
++
+Set `$maxload` to undefined value (`undef`) to turn this feature off.
+[Default: 300]
+
+$per_request_config::
+	If this is set to code reference, it will be run once for each request.
+	You can	set parts of configuration that change per session this way.
+	For example, one might use the following code in a gitweb configuration
+	file
++
+--------------------------------------------------------------------------------
+our $per_request_config = sub {
+	$ENV{GL_USER} = $cgi->remote_user || "gitweb";
+};
+--------------------------------------------------------------------------------
++
+If `$per_request_config` is not a code reference, it is interpreted as boolean
+value.  If it is true gitweb will process config files once per request,
+and if it is false gitweb will process config files only once, each time it
+is executed.  [Default: true]
++
+*NOTE*: `$my_url`, `$my_uri`, and `$base_url` are overwritten with their default
+values before every request, so if you want to change them, be sure to set
+this variable to true or a code reference effecting the desired changes.
++
+This variable matters only when using persistent web environments that
+serve multiple requests using single gitweb instance, like mod_perl,
+FastCGI or Plackup.
+
+
+Other variables
+~~~~~~~~~~~~~~~
+Usually you should not need to change (adjust) any of configuration
+variables described below; they should be automatically set by gitweb to
+correct value.
+
+
+$version::
+	Gitweb version, set automatically when creating gitweb.cgi from
+	gitweb.perl. You might want to modify it if you are running modified
+	gitweb, for example
++
+---------------------------------------------------
+our $version .= " with caching";
+---------------------------------------------------
++
+if you run modified version of gitweb with caching support.  This variable
+is purely informational, used e.g. in the "generator" meta header in HTML
+header.
+
+$my_url::
+$my_uri::
+	Full URL and absolute URL of the gitweb script;
+	in earlier versions of gitweb you might have need to set those
+	variables, but now there should be no need to do it.  See
+	`$per_request_config` if you need to set them still.
+
+$base_url::
+	Base URL for relative URLs in pages generated by gitweb,
+	(e.g. `$logo`, `$favicon`, `@stylesheets` if they are relative URLs),
+	needed and used '<base href="$base_url">' only for URLs with nonempty
+	PATH_INFO.  Usually gitweb sets its value correctly,
+	and there is no need to set this variable, e.g. to $my_uri or "/".
+	See `$per_request_config` if you need to override it anyway.
+
+
+CONFIGURING GITWEB FEATURES
+---------------------------
+Many gitweb features can be enabled (or disabled) and configured using the
+`%feature` hash.  Names of gitweb features are keys of this hash.
+
+Each `%feature` hash element is a hash reference and has the following
+structure:
+----------------------------------------------------------------------
+"<feature_name>" => {
+	"sub" => <feature-sub (subroutine)>,
+	"override" => <allow-override (boolean)>,
+	"default" => [ <options>... ]
+},
+----------------------------------------------------------------------
+Some features cannot be overridden per project.  For those
+features the structure of appropriate `%feature` hash element has a simpler
+form:
+----------------------------------------------------------------------
+"<feature_name>" => {
+	"override" => 0,
+	"default" => [ <options>... ]
+},
+----------------------------------------------------------------------
+As one can see it lacks the \'sub' element.
+
+The meaning of each part of feature configuration is described
+below:
+
+default::
+	List (array reference) of feature parameters (if there are any),
+	used also to toggle (enable or disable) given feature.
++
+Note that it is currently *always* an array reference, even if
+feature doesn't accept any configuration parameters, and \'default'
+is used only to turn it on or off.  In such case you turn feature on
+by setting this element to `[1]`, and torn it off by setting it to
+`[0]`.  See also the passage about the "blame" feature in the "Examples"
+section.
++
+To disable features that accept parameters (are configurable), you
+need to set this element to empty list i.e. `[]`.
+
+override::
+	If this field has a true value then the given feature is
+	overriddable, which means that it can be configured
+	(or enabled/disabled) on a per-repository basis.
++
+Usually given "<feature>" is configurable via the `gitweb.<feature>`
+config variable in the per-repository git configuration file.
++
+*Note* that no feature is overriddable by default.
+
+sub::
+	Internal detail of implementation.  What is important is that
+	if this field is not present then per-repository override for
+	given feature is not supported.
++
+You wouldn't need to ever change it in gitweb config file.
+
+
+Features in `%feature`
+~~~~~~~~~~~~~~~~~~~~~~
+The gitweb features that are configurable via `%feature` hash are listed
+below.  This should be a complete list, but ultimately the authoritative
+and complete list is in gitweb.cgi source code, with features described
+in the comments.
+
+blame::
+	Enable the "blame" and "blame_incremental" blob views, showing for
+	each line the last commit that modified it; see linkgit:git-blame[1].
+	This can be very CPU-intensive and is therefore disabled by default.
++
+This feature can be configured on a per-repository basis via
+repository's `gitweb.blame` configuration variable (boolean).
+
+snapshot::
+	Enable and configure the "snapshot" action, which allows user to
+	download a compressed archive of any tree or commit, as produced
+	by linkgit:git-archive[1] and possibly additionally compressed.
+	This can potentially generate high traffic if you have large project.
++
+The value of \'default' is a list of names of snapshot formats,
+defined in `%known_snapshot_formats` hash, that you wish to offer.
+Supported formats include "tgz", "tbz2", "txz" (gzip/bzip2/xz
+compressed tar archive) and "zip"; please consult gitweb sources for
+a definitive list.  By default only "tgz" is offered.
++
+This feature can be configured on a per-repository basis via
+repository's `gitweb.blame` configuration variable, which contains
+a comma separated list of formats or "none" to disable snapshots.
+Unknown values are ignored.
+
+grep::
+	Enable grep search, which lists the files in currently selected
+	tree (directory) containing the given string; see linkgit:git-grep[1].
+	This can be potentially CPU-intensive, of course.  Enabled by default.
++
+This feature can be configured on a per-repository basis via
+repository's `gitweb.grep` configuration variable (boolean).
+
+pickaxe::
+	Enable the so called pickaxe search, which will list the commits
+	that introduced or removed a given string in a file.  This can be
+	practical and quite faster alternative to "blame" action, but it is
+	still potentially CPU-intensive.  Enabled by default.
++
+The pickaxe search is described in linkgit:git-log[1] (the
+description of `-S<string>` option, which refers to pickaxe entry in
+linkgit:gitdiffcore[7] for more details).
++
+This feature can be configured on a per-repository basis by setting
+repository's `gitweb.pickaxe` configuration variable (boolean).
+
+show-sizes::
+	Enable showing size of blobs (ordinary files) in a "tree" view, in a
+	separate column, similar to what `ls -l` does; see description of
+	`-l` option in linkgit:git-ls-tree[1] manpage.  This costs a bit of
+	I/O.  Enabled by default.
++
+This feature can be configured on a per-repository basis via
+repository's `gitweb.showsizes` configuration variable (boolean).
+
+patches::
+	Enable and configure "patches" view, which displays list of commits in email
+	(plain text) output format; see also linkgit:git-format-patch[1].
+	The value is the maximum number of patches in a patchset generated
+	in "patches" view.  Set the 'default' field to a list containing single
+	item of or to an empty list to disable patch view, or to a list
+	containing a single negative number to remove any limit.
+	Default value is 16.
++
+This feature can be configured on a per-repository basis via
+repository's `gitweb.patches` configuration variable (integer).
+
+avatar::
+	Avatar support.  When this feature is enabled, views such as
+	"shortlog" or "commit" will display an avatar associated with
+	the email of each committer and author.
++
+Currently available providers are *"gravatar"* and *"picon"*.
+Only one provider at a time can be selected ('default' is one element list).
+If an unknown provider is specified, the feature is disabled.
+*Note* that some providers might require extra Perl packages to be
+installed; see 'gitweb/INSTALL' for more details.
++
+This feature can be configured on a per-repository basis via
+repository's `gitweb.avatar` configuration variable.
++
+See also `%avatar_size` with pixel sizes for icons and avatars
+("default" is used for one-line like "log" and "shortlog", "double"
+is used for two-line like "commit", "commitdiff" or "tag").  If the
+default font sizes or lineheights are changed (e.g. via adding extra
+CSS stylesheet in `@stylesheets`), it may be appropriate to change
+these values.
+
+highlight::
+	Server-side syntax highlight support in "blob" view.  It requires
+	`$highlight_bin` program to be available (see the description of
+	this variable in the "Configuration variables" section above),
+	and therefore is disabled by default.
++
+This feature can be configured on a per-repository basis via
+repository's `gitweb.highlight` configuration variable (boolean).
+
+remote_heads::
+	Enable displaying remote heads (remote-tracking branches) in the "heads"
+	list.  In most cases the list of remote-tracking branches is an
+	unnecessary internal private detail, and this feature is therefore
+	disabled by default.  linkgit:git-instaweb[1], which is usually used
+	to browse local repositories, enables and uses this feature.
++
+This feature can be configured on a per-repository basis via
+repository's `gitweb.remote_heads` configuration variable (boolean).
+
+
+The remaining features cannot be overridden on a per project basis.
+
+search::
+	Enable text search, which will list the commits which match author,
+	committer or commit text to a given string; see the description of
+	`--author`, `--committer` and `--grep` options in linkgit:git-log[1]
+	manpage.  Enabled by default.
++
+Project specific override is not supported.
+
+forks::
+	If this feature is enabled, gitweb considers projects in
+	subdirectories of project root (basename) to be forks of existing
+	projects.  For each project `$projname.git`, projects in the
+	`$projname/` directory and its subdirectories will not be
+	shown in the main projects list.  Instead, a \'+' mark is shown
+	next to `$projname`, which links to a "forks" view that lists all
+	the forks (all projects in `$projname/` subdirectory).  Additionally
+	a "forks" view for a project is linked from project summary page.
++
+If the project list is taken from a file (`$projects_list` points to a
+file), forks are only recognized if they are listed after the main project
+in that file.
++
+Project specific override is not supported.
+
+actions::
+	Insert custom links to the action bar of all project pages.  This
+	allows you to link to third-party scripts integrating into gitweb.
++
+The "default" value consists of a list of triplets in the form
+`("<label>", "<link>", "<position>")` where "position" is the label
+after which to insert the link, "link" is a format string where `%n`
+expands to the project name, `%f` to the project path within the
+filesystem (i.e. "$projectroot/$project"), `%h` to the current hash
+(\'h' gitweb parameter) and `%b` to the current hash base
+(\'hb' gitweb parameter); `%%` expands to \'%'.
++
+For example, at the time this page was written, the http://repo.or.cz[]
+git hosting site set it to the following to enable graphical log
+(using the third party tool *git-browser*):
++
+----------------------------------------------------------------------
+$feature{'actions'}{'default'} =
+	[ ('graphiclog', '/git-browser/by-commit.html?r=%n', 'summary')];
+----------------------------------------------------------------------
++
+This adds a link titled "graphiclog" after the "summary" link, leading to
+`git-browser` script, passing `r=<project>` as a query parameter.
++
+Project specific override is not supported.
+
+timed::
+	Enable displaying how much time and how many git commands it took to
+	generate and display each page in the page footer (at the bottom of
+	page).  For example the footer might contain: "This page took 6.53325
+	seconds and 13 git commands to generate."  Disabled by default.
++
+Project specific override is not supported.
+
+javascript-timezone::
+	Enable and configure the ability to change a common timezone for dates
+	in gitweb output via JavaScript.  Dates in gitweb output include
+	authordate and committerdate in "commit", "commitdiff" and "log"
+	views, and taggerdate in "tag" view.  Enabled by default.
++
+The value is a list of three values: a default timezone (for if the client
+hasn't selected some other timezone and saved it in a cookie), a name of cookie
+where to store selected timezone, and a CSS class used to mark up
+dates for manipulation.  If you want to turn this feature off, set "default"
+to empty list: `[]`.
++
+Typical gitweb config files will only change starting (default) timezone,
+and leave other elements at their default values:
++
+---------------------------------------------------------------------------
+$feature{'javascript-timezone'}{'default'}[0] = "utc";
+---------------------------------------------------------------------------
++
+The example configuration presented here is guaranteed to be backwards
+and forward compatible.
++
+Timezone values can be "local" (for local timezone that browser uses), "utc"
+(what gitweb uses when JavaScript or this feature is disabled), or numerical
+timezones in the form of "+/-HHMM", such as "+0200".
++
+Project specific override is not supported.
+
+
+EXAMPLES
+--------
+
+To enable blame, pickaxe search, and snapshot support (allowing "tar.gz" and
+"zip" snapshots), while allowing individual projects to turn them off, put
+the following in your GITWEB_CONFIG file:
+
+        $feature{'blame'}{'default'} = [1];
+        $feature{'blame'}{'override'} = 1;
+
+        $feature{'pickaxe'}{'default'} = [1];
+        $feature{'pickaxe'}{'override'} = 1;
+
+        $feature{'snapshot'}{'default'} = ['zip', 'tgz'];
+        $feature{'snapshot'}{'override'} = 1;
+
+If you allow overriding for the snapshot feature, you can specify which
+snapshot formats are globally disabled. You can also add any command line
+options you want (such as setting the compression level). For instance, you
+can disable Zip compressed snapshots and set *gzip*(1) to run at level 6 by
+adding the following lines to your gitweb configuration file:
+
+        $known_snapshot_formats{'zip'}{'disabled'} = 1;
+        $known_snapshot_formats{'tgz'}{'compressor'} = ['gzip','-6'];
+
+ENVIRONMENT
+-----------
+The location of per-instance and system-wide configuration files can be
+overridden using the following environment variables:
+
+GITWEB_CONFIG::
+	Sets location of per-instance configuration file.
+GITWEB_CONFIG_SYSTEM::
+	Sets location of fallback system-wide configuration file.
+	This file is read only if per-instance one does not exist.
+GITWEB_CONFIG_COMMON::
+	Sets location of common system-wide configuration file.
+
+
+FILES
+-----
+gitweb_config.perl::
+	This is default name of per-instance configuration file.  The
+	format of this file is described above.
+/etc/gitweb.conf::
+	This is default name of fallback system-wide configuration
+	file.  This file is used only if per-instance configuration
+	variable is not found.
+/etc/gitweb-common.conf::
+	This is default name of common system-wide configuration
+	file.
+
+
+SEE ALSO
+--------
+linkgit:gitweb[1], linkgit:git-instaweb[1]
+
+'gitweb/README', 'gitweb/INSTALL'
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/gitweb/README b/gitweb/README
index a998820..cf528d3 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -41,140 +41,9 @@ Ultimate description on how to reconfigure the default features setting
 in your `GITWEB_CONFIG` or per-project in `project.git/config` can be found
 as comments inside 'gitweb.cgi'.
 
-See also the "Gitweb config file" (with an example of config file), and
-the "Gitweb repositories" sections in INSTALL file for gitweb.
+See also gitweb.conf(5) manpage.
 
 
-The gitweb config file is a fragment of perl code. You can set variables
-using "our $variable = value"; text from "#" character until the end
-of a line is ignored. See perlsyn(1) man page for details.
-
-Below is the list of variables which you might want to set in gitweb config.
-See the top of 'gitweb.cgi' for the full list of variables and their
-descriptions.
-
-Gitweb config file variables
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-You can set, among others, the following variables in gitweb config files
-(with the exception of $projectroot and $projects_list this list does
-not include variables usually directly set during build):
- * $GIT
-   Core git executable to use.  By default set to "$GIT_BINDIR/git", which
-   in turn is by default set to "$(bindir)/git".  If you use git from binary
-   package, set this to "/usr/bin/git".  This can just be "git" if your
-   webserver has a sensible PATH.  If you have multiple git versions
-   installed it can be used to choose which one to use.
- * $version
-   Gitweb version, set automatically when creating gitweb.cgi from
-   gitweb.perl. You might want to modify it if you are running modified
-   gitweb.
- * $projectroot
-   Absolute filesystem path which will be prepended to project path;
-   the path to repository is $projectroot/$project.  Set to
-   $GITWEB_PROJECTROOT during installation.  This variable have to be
-   set correctly for gitweb to find repositories.
- * $projects_list
-   Source of projects list, either directory to scan, or text file
-   with list of repositories (in the "<URI-encoded repository path> SP
-   <URI-encoded repository owner>" line format; actually there can be
-   any sequence of whitespace in place of space (SP)).  Set to
-   $GITWEB_LIST during installation.  If empty, $projectroot is used
-   to scan for repositories.
- * $my_url, $my_uri
-   Full URL and absolute URL of gitweb script;
-   in earlier versions of gitweb you might have need to set those
-   variables, now there should be no need to do it.  See
-   $per_request_config if you need to set them still.
- * $base_url
-   Base URL for relative URLs in pages generated by gitweb,
-   (e.g. $logo, $favicon, @stylesheets if they are relative URLs),
-   needed and used only for URLs with nonempty PATH_INFO via
-   <base href="$base_url">.  Usually gitweb sets its value correctly,
-   and there is no need to set this variable, e.g. to $my_uri or "/".
-   See $per_request_config if you need to set it anyway.
- * $home_link
-   Target of the home link on top of all pages (the first part of view
-   "breadcrumbs").  By default set to absolute URI of a page ($my_uri).
- * @stylesheets
-   List of URIs of stylesheets (relative to base URI of a page). You
-   might specify more than one stylesheet, for example use gitweb.css
-   as base, with site specific modifications in separate stylesheet
-   to make it easier to upgrade gitweb. You can add 'site' stylesheet
-   for example by using
-      push @stylesheets, "gitweb-site.css";
-   in the gitweb config file.
- * $logo_url, $logo_label
-   URI and label (title) of GIT logo link (or your site logo, if you choose
-   to use different logo image). By default they point to git homepage;
-   in the past they pointed to git documentation at www.kernel.org.
- * $projects_list_description_width
-   The width (in characters) of the projects list "Description" column.
-   Longer descriptions will be cut (trying to cut at word boundary);
-   full description is available as 'title' attribute (usually shown on
-   mouseover).  By default set to 25, which might be too small if you
-   use long project descriptions.
- * $projects_list_group_categories
-   Enables the grouping of projects by category on the project list page.
-   The category of a project is determined by the $GIT_DIR/category
-   file or the 'gitweb.category' variable in its repository configuration.
-   Disabled by default.
- * $project_list_default_category
-   Default category for projects for which none is specified.  If set
-   to the empty string, such projects will remain uncategorized and
-   listed at the top, above categorized projects.
- * @git_base_url_list
-   List of git base URLs used for URL to where fetch project from, shown
-   in project summary page.  Full URL is "$git_base_url/$project".
-   You can setup multiple base URLs (for example one for  git:// protocol
-   access, and one for http:// "dumb" protocol access).  Note that per
-   repository configuration in 'cloneurl' file, or as values of gitweb.url
-   project config.
- * $default_blob_plain_mimetype
-   Default mimetype for blob_plain (raw) view, if mimetype checking
-   doesn't result in some other type; by default 'text/plain'.
- * $default_text_plain_charset
-   Default charset for text files. If not set, web server configuration
-   would be used.
- * $mimetypes_file
-   File to use for (filename extension based) guessing of MIME types before
-   trying /etc/mime.types. Path, if relative, is taken currently as
-   relative to the current git repository.
- * $fallback_encoding
-   Gitweb assumes this charset if line contains non-UTF-8 characters.
-   Fallback decoding is used without error checking, so it can be even
-   'utf-8'. Value must be valid encoding; see Encoding::Supported(3pm) man
-   page for a list.   By default 'latin1', aka. 'iso-8859-1'.
- * @diff_opts
-   Rename detection options for git-diff and git-diff-tree. By default
-   ('-M'); set it to ('-C') or ('-C', '-C') to also detect copies, or
-   set it to () if you don't want to have renames detection.
- * $prevent_xss
-   If true, some gitweb features are disabled to prevent content in
-   repositories from launching cross-site scripting (XSS) attacks.  Set this
-   to true if you don't trust the content of your repositories. The default
-   is false.
- * $maxload
-   Used to set the maximum load that we will still respond to gitweb queries.
-   If server load exceed this value then return "503 Service Unavailable" error.
-   Server load is taken to be 0 if gitweb cannot determine its value.  Set it to
-   undefined value to turn it off.  The default is 300.
- * $highlight_bin
-   Path to the highlight executable to use (must be the one from
-   http://www.andre-simon.de due to assumptions about parameters and output).
-   Useful if highlight is not installed on your webserver's PATH.
-   [Default: highlight]
- * $per_request_config
-   If set to code reference, it would be run once per each request.  You can
-   set parts of configuration that change per session, e.g. by setting it to
-     sub { $ENV{GL_USER} = $cgi->remote_user || "gitweb"; }
-   Otherwise it is treated as boolean value: if true gitweb would process
-   config file once per request, if false it would process config file only
-   once.  Note: $my_url, $my_uri, and $base_url are overwritten with
-   their default values before every request, so if you want to change
-   them, be sure to set this variable to true or a code reference effecting
-   the desired changes.  The default is true.
-
 Projects list file format
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
1.7.6

^ permalink raw reply related

* [PATCHv5/RFC 1/6] Documentation: Preparation for gitweb manpages
From: Jakub Narebski @ 2011-10-08 18:31 UTC (permalink / raw)
  To: git, Drew Northup; +Cc: Jonathan Nieder, Jakub Narebski
In-Reply-To: <1318098723-12813-1-git-send-email-jnareb@gmail.com>

Gitweb documentation currently consists of gitweb/README, gitweb/INSTALL
and comments in gitweb source code.  This is harder to find, use and
browse than manpages ("man gitweb" or "git help gitweb") and HTML
documentation ("git help --web gitweb").

The goal of the next two commits is to move documentation out of
gitweb/README to gitweb.txt and gitweb.conf.txt manpages, reducing its
size 10x from around 500 to around 50 lines (two pages), and to move
information not related directly to building and installing gitweb out
of gitweb/INSTALL.

The idea is to have the gitweb manpage sources reside in AsciiDoc
format in the Documentation/ directory, like for gitk and git-gui.
This means that building git documentation (with "make doc") would
also build gitweb manpages.

An alternate solution would be to have gitweb documentation in the
gitweb/ directory, perhaps in POD format (see perlpod(1)).


This patch adds infrastructure for easy generation of only
gitweb-related manpages.  It adds a currently empty 'gitweb-doc'
target to Documentation/Makefile, and a 'doc' proxy target to
gitweb/Makefile.

This way to build only gitweb documentation in both 'man' and 'html'
formats one can use

  make -C gitweb doc

or

  cd gitweb; make doc

This somewhat follows the idea of 'full-svn-test' and 'gitweb-test' in
t/Makefile.  Note also that with alternate solution, where source of
gitweb manpages would reside in the gitweb/ directory, "make doc"
would invoke "make -C gitweb doc" to generate formatted docs.

The gitweb.conf(5) and gitweb(1) manpages will be added in subsequent
commits.

[Commit message improved with help of Jonathan Nieder]

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This commit is not strictly necessary: it only adds "doc" target to
gitweb/Makefile, and "gitweb-doc" target to Documentation/Makefile;
neither is run when e.g. generating RPM.

They are here because they would be here if documentation source was
kept along with gitweb script in the 'gitweb/' subdirectory, and to
make it easier and faster to test the changes.

 Documentation/Makefile |    3 +++
 gitweb/Makefile        |    7 ++++++-
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 6346a75..44be67b 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -170,6 +170,9 @@ info: git.info gitman.info
 
 pdf: user-manual.pdf
 
+GITWEB_DOC = $(filter gitweb.%,$(DOC_HTML) $(DOC_MAN1) $(DOC_MAN5) $(DOC_MAN7))
+gitweb-doc: $(GITWEB_DOC)
+
 install: install-man
 
 install-man: man
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 1c85b5f..3014d80 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -174,6 +174,11 @@ test-installed:
 	GITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \
 		$(MAKE) -C ../t gitweb-test
 
+### Documentation
+
+doc:
+	$(MAKE) -C ../Documentation gitweb-doc
+
 ### Installation rules
 
 install: all
@@ -187,5 +192,5 @@ install: all
 clean:
 	$(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS
 
-.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE
+.PHONY: all clean install test test-installed doc .FORCE-GIT-VERSION-FILE FORCE
 
-- 
1.7.6

^ permalink raw reply related

* [PATCHv5/RFC 0/6] Moving gitweb documentation to manpages
From: Jakub Narebski @ 2011-10-08 18:31 UTC (permalink / raw)
  To: git, Drew Northup; +Cc: Jonathan Nieder, Jakub Narebski

The original patch adding manpage for /etc/gitweb.conf was sent by
Drew Northup (as can be seen from shortlog); I have added manpage for
gitweb itself, inspired by his patch.  Unfortunately Drew doesn't
currently have time to work on this patch (patch series), so that is
why it is me resending this series (again).

Compared to previous version

  "[PATCH/RFCv4 0/4] Moving gitweb documentation to manpages"
  http://thread.gmane.org/gmane.comp.version-control.git/181605

it has much improved gitweb.conf.txt manpage, and "Documentation:
Preparation for gitweb manpages" commit message, thanks to feedback
provided by Jonathan Nieder.

New in this series are patches 4 and 5, which add links to the newly
created gitweb documentation and gitweb config variables from other
manpages, respectively.

Note that only "Documentation: Preparation for gitweb manpages" (which
is not strictly necessary) and "gitweb: Add manpage for gitweb" (which
didn't get as much review as "gitweb: Add manpage for gitweb
configuration files" are still marked as RFC.

I have checked that RPM generation of 'gitweb' package works correctly
wrt. gitweb documentation.

Table of contents:
~~~~~~~~~~~~~~~~~~
 [PATCHv5/RFC 1/6] Documentation: Preparation for gitweb manpages
 [PATCHv5 2/6] gitweb: Add manpage for gitweb configuration files
 [PATCHv5/RFC 3/6] gitweb: Add manpage for gitweb
 [PATCHv5 4/6] Documentation: Link to gitweb(1) and 
   gitweb.conf(5) in other manpages
 [PATCHv5 5/6] Documentation: Add gitweb config variables to
   git-config(1)
 [PATCHv5 6/6] gitweb: Add gitweb manpages to 'gitweb' package in
   git.spec.in

Shortlog:
~~~~~~~~~
Drew Northup (1):
  gitweb: Add manpage for gitweb configuration files

Jakub Narebski (5):
  Documentation: Preparation for gitweb manpages
  gitweb: Add manpage for gitweb
  Documentation: Link to gitweb(1) and gitweb.conf(5) in other manpages
  Documentation: Add gitweb config variables to git-config(1)
  gitweb: Add gitweb manpages to 'gitweb' package in git.spec

Diffstat:
~~~~~~~~~
 Documentation/Makefile         |    7 +-
 Documentation/config.txt       |   17 +
 Documentation/git-instaweb.txt |    4 +
 Documentation/gitweb.conf.txt  |  875 ++++++++++++++++++++++++++++++++++++++++
 Documentation/gitweb.txt       |  703 ++++++++++++++++++++++++++++++++
 command-list.txt               |    1 +
 git.spec.in                    |    7 +
 gitweb/INSTALL                 |   94 +----
 gitweb/Makefile                |    7 +-
 gitweb/README                  |  411 +------------------
 10 files changed, 1647 insertions(+), 479 deletions(-)
 create mode 100644 Documentation/gitweb.conf.txt
 create mode 100644 Documentation/gitweb.txt

-- 
1.7.6

^ permalink raw reply

* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Jay Soffian @ 2011-10-08 18:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Todd A. Jacobs
In-Reply-To: <7vfwj4tplw.fsf@alter.siamese.dyndns.org>

On Fri, Oct 7, 2011 at 6:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Think and look forward.
>
> You are complaining that the "commit" does not know enough to behave as if
> it were a part of the merge command workflow if you split a usual merge
> into two steps "merge --no-commit; commit".

No Junio, you have my argument completely reversed.

I am complaining that git-merge implements commits internally, which
gives it unique behavior from git-{commit/cherry-pick/revert} (the
latter two of which just run external git-commit). I'm saying merge is
fundamentally broken to do it this way. And maybe that's something
that should be fixed in 2.0 -- that git-merge should just call out to
git-commit, just like cherry-pick/revert do.

In case that's not clear: I think that git-merge should eventually
behave identically to "merge --no-commit; commit".

> How would you make it better? Would you strip all the things usual "merge"
> does, so that it would work identically to the split one,

Yes.

> losing some hook support and such.

Yes, I would lose the post-merge hook and such.

>, or would you rather make the split case work similar to the usual merge?

No, I would not do that.

BTW, the same arguments apply to git-am, which uses git-commit-tree,
and so implements its own set of hooks.

> I'd say between "merge" and "merge --no-commit ; commit", the latter is
> what needs to be fixed. Viewed that way, why would you even consider
> making the new option behave similar to the _wrong_ one?

Strongly disagree. I think it would make much more sense for all
commits to flow through git-commit, which would ensure consistent
behavior. I think we've got a mishmash of hooks which evolved over
time.

>> I didn't bother with the commit status, it's more code than I wanted
>> to deal with duplicating/refactoring from commit.c.
>
> What do you mean by "commit status"? If you mean this patch is incomplete,
> it would have been nicer if it were labeled with [PATCH/RFC].

No, I meant that git-commit includes status information about the
commit itself as comments in the commit message (git config
commit.status), and I didn't implement that. I don't think that makes
this patch incomplete however, that could be added by a later patch.

I'll send another iteration with your comments below addressed.

j.

>> diff --git a/builtin/merge.c b/builtin/merge.c
>> index ee56974371..0dee53b7e4 100644
>> --- a/builtin/merge.c
>> +++ b/builtin/merge.c
>> @@ -46,6 +46,7 @@ static const char * const builtin_merge_usage[] = {
>>
>>  static int show_diffstat = 1, shortlog_len, squash;
>>  static int option_commit = 1, allow_fast_forward = 1;
>> +static int option_edit = 0;
>
> No need to move this into .data segment when it can be in .bss
> segment. Drop the unnecessary " = 0" before ";".
>
>> @@ -842,30 +845,54 @@ static void add_strategies(const char *string, unsigned attr)
>>
>>  }
>>
>> -static void write_merge_msg(void)
>> +static void write_merge_msg(struct strbuf *msg)
>>  {
>>       int fd = open(git_path("MERGE_MSG"), O_WRONLY | O_CREAT, 0666);
>>       if (fd < 0)
>>               die_errno(_("Could not open '%s' for writing"),
>>                         git_path("MERGE_MSG"));
>> -     if (write_in_full(fd, merge_msg.buf, merge_msg.len) != merge_msg.len)
>> +     if (write_in_full(fd, msg->buf, msg->len) != msg->len)
>>               die_errno(_("Could not write to '%s'"), git_path("MERGE_MSG"));
>>       close(fd);
>>  }
>>
>> -static void read_merge_msg(void)
>> +static void read_merge_msg(struct strbuf *msg)
>>  {
>> -     strbuf_reset(&merge_msg);
>> -     if (strbuf_read_file(&merge_msg, git_path("MERGE_MSG"), 0) < 0)
>> +     strbuf_reset(msg);
>> +     if (strbuf_read_file(msg, git_path("MERGE_MSG"), 0) < 0)
>>               die_errno(_("Could not read from '%s'"), git_path("MERGE_MSG"));
>>  }
>>
>> -static void run_prepare_commit_msg(void)
>> +static void write_merge_state();
>
> s/()/(void)/;
>
> Thanks.
>
>

^ permalink raw reply

* Re: unexpected behavior with `git log --skip filename`
From: Jay Soffian @ 2011-10-08 17:47 UTC (permalink / raw)
  To: Andrew McNabb; +Cc: git
In-Reply-To: <20111008023637.GA18136@mcnabbs.org>

On Fri, Oct 7, 2011 at 10:36 PM, Andrew McNabb <amcnabb@mcnabbs.org> wrote:
> I went back to reproduce this, and I think I may have been using the
> --follow option earlier.

--follow is, er, special. It doesn't combine well with other rev-list
options. You'll have to search this list for details. Sorry I can't
provide more info. :-(

j.

^ permalink raw reply

* [PATCH 6/6] revert: Simplify passing command-line arguments around
From: Ramkumar Ramachandra @ 2011-10-08 17:36 UTC (permalink / raw)
  To: Git List
  Cc: Jonathan Nieder, Junio C Hamano, Jeff King, Daniel Barkalow,
	Christian Couder
In-Reply-To: <1318095407-26429-1-git-send-email-artagnon@gmail.com>

From: Jonathan Nieder <jrnieder@gmail.com>

The chief command-line argument parser, parse_args, currently parses
arguments into an (argc, argv) and lets prepare_revs, a later
function, turn it into a structure.  Change this so that callers of
the cherry-pick machinery will have to fill in an options structure
instead of crafting command-line arguments.

[rr: minor improvements, commit message]

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/revert.c |   53 +++++++++++++++++++++++++++++------------------------
 1 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index a9dd210..a72b20d 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,13 +54,14 @@ struct replay_opts {
 	int allow_rerere_auto;
 
 	int mainline;
-	int commit_argc;
-	const char **commit_argv;
 
 	/* Merge strategy */
 	const char *strategy;
 	const char **xopts;
 	size_t xopts_nr, xopts_alloc;
+
+	/* Only used by REPLAY_NONE */
+	struct rev_info *revs;
 };
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -163,9 +164,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 			die(_("program error"));
 	}
 
-	opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
-					PARSE_OPT_KEEP_ARGV0 |
-					PARSE_OPT_KEEP_UNKNOWN);
+	argc = parse_options(argc, argv, NULL, options, usage_str,
+			PARSE_OPT_KEEP_ARGV0 |
+			PARSE_OPT_KEEP_UNKNOWN);
 
 	/* Check for incompatible subcommands */
 	verify_opt_mutually_compatible(me,
@@ -200,9 +201,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 				NULL);
 	}
 
-	else if (opts->commit_argc < 2)
-		usage_with_options(usage_str, options);
-
 	if (opts->allow_ff)
 		verify_opt_compatible(me, "--ff",
 				"--signoff", opts->signoff,
@@ -210,7 +208,20 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 				"-x", opts->record_origin,
 				"--edit", opts->edit,
 				NULL);
-	opts->commit_argv = argv;
+
+	if (opts->subcommand == REPLAY_NONE) {
+		opts->revs = xmalloc(sizeof(*opts->revs));
+		init_revisions(opts->revs, NULL);
+		opts->revs->no_walk = 1;
+		if (argc < 2)
+			usage_with_options(usage_str, options);
+		argc = setup_revisions(argc, argv, opts->revs, NULL);
+	} else
+		opts->revs = NULL;
+
+	/* Forbid stray command-line arguments */
+	if (argc > 1)
+		usage_with_options(usage_str, options);
 }
 
 struct commit_message {
@@ -607,23 +618,15 @@ static int do_pick_commit(struct commit *commit, enum replay_action action,
 	return res;
 }
 
-static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
+static void prepare_revs(struct replay_opts *opts)
 {
-	int argc;
-
-	init_revisions(revs, NULL);
-	revs->no_walk = 1;
 	if (opts->action != REPLAY_REVERT)
-		revs->reverse = 1;
+		opts->revs->reverse ^= 1;
 
-	argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
-	if (argc > 1)
-		usage(*revert_or_cherry_pick_usage(opts));
-
-	if (prepare_revision_walk(revs))
+	if (prepare_revision_walk(opts->revs))
 		die(_("revision walk setup failed"));
 
-	if (!revs->commits)
+	if (!opts->revs->commits)
 		die(_("empty commit set passed"));
 }
 
@@ -817,14 +820,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
 static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
 				struct replay_opts *opts)
 {
-	struct rev_info revs;
 	struct commit *commit;
 	struct replay_insn_list **next;
 
-	prepare_revs(&revs, opts);
+	prepare_revs(opts);
 
 	next = todo_list;
-	while ((commit = get_revision(&revs)))
+	while ((commit = get_revision(opts->revs)))
 		next = replay_insn_list_append(opts->action, commit, next);
 }
 
@@ -948,6 +950,9 @@ static int pick_revisions(struct replay_opts *opts)
 	struct replay_insn_list *todo_list = NULL;
 	unsigned char sha1[20];
 
+	if (opts->subcommand == REPLAY_NONE)
+		assert(opts->revs);
+
 	read_and_refresh_cache(opts);
 
 	/*
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 5/6] revert: Allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-10-08 17:36 UTC (permalink / raw)
  To: Git List
  Cc: Jonathan Nieder, Junio C Hamano, Jeff King, Daniel Barkalow,
	Christian Couder
In-Reply-To: <1318095407-26429-1-git-send-email-artagnon@gmail.com>

Parse the instruction list in '.git/sequencer/todo' as a list of
(action, operand) pairs, instead of assuming all instructions use the
same action.  So now, you can do:

  pick fdc0b12 picked
  revert 965fed4 anotherpick

For cherry-pick and revert, this means that a 'git cherry-pick
--continue' can continue an ongoing revert operation and viceversa.
This patch lays the foundation for extending the parser to support
more actions so 'git rebase -i' can reuse this machinery in the
future.  While at it, also improve the error messages reported by the
insn sheet parser.

Helped-by: Jonathan Nieder <jrnider@gmail.com>
Acked-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c                |  138 +++++++++++++++++++-------------------
 sequencer.h                     |    8 ++
 t/t3510-cherry-pick-sequence.sh |   58 ++++++++++++++++
 3 files changed, 135 insertions(+), 69 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index aa6c34e..a9dd210 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,6 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-enum replay_action { REVERT, CHERRY_PICK };
 enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
 
 struct replay_opts {
@@ -68,14 +67,14 @@ struct replay_opts {
 
 static const char *action_name(const struct replay_opts *opts)
 {
-	return opts->action == REVERT ? "revert" : "cherry-pick";
+	return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
 }
 
 static char *get_encoding(const char *message);
 
 static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
 {
-	return opts->action == REVERT ? revert_usage : cherry_pick_usage;
+	return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
 }
 
 static int option_parse_x(const struct option *opt,
@@ -154,7 +153,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		OPT_END(),
 	};
 
-	if (opts->action == CHERRY_PICK) {
+	if (opts->action == REPLAY_PICK) {
 		struct option cp_extra[] = {
 			OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
 			OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
@@ -348,7 +347,7 @@ static int error_dirty_index(struct replay_opts *opts)
 		return error_resolve_conflict(action_name(opts));
 
 	/* Different translation strings for cherry-pick and revert */
-	if (opts->action == CHERRY_PICK)
+	if (opts->action == REPLAY_PICK)
 		error(_("Your local changes would be overwritten by cherry-pick."));
 	else
 		error(_("Your local changes would be overwritten by revert."));
@@ -452,7 +451,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
 	return run_command_v_opt(args, RUN_GIT_CMD);
 }
 
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+static int do_pick_commit(struct commit *commit, enum replay_action action,
+			struct replay_opts *opts)
 {
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
@@ -527,7 +527,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 
 	defmsg = git_pathdup("MERGE_MSG");
 
-	if (opts->action == REVERT) {
+	if (action == REPLAY_REVERT) {
 		base = commit;
 		base_label = msg.label;
 		next = parent;
@@ -570,7 +570,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 			write_cherry_pick_head(commit);
 	}
 
-	if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REVERT) {
+	if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
 		res = do_recursive_merge(base, next, base_label, next_label,
 					 head, &msgbuf, opts);
 		write_message(&msgbuf, defmsg);
@@ -589,7 +589,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 	}
 
 	if (res) {
-		error(opts->action == REVERT
+		error(action == REPLAY_REVERT
 		      ? _("could not revert %s... %s")
 		      : _("could not apply %s... %s"),
 		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -613,7 +613,7 @@ static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
 
 	init_revisions(revs, NULL);
 	revs->no_walk = 1;
-	if (opts->action != REVERT)
+	if (opts->action != REPLAY_REVERT)
 		revs->reverse = 1;
 
 	argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
@@ -659,87 +659,87 @@ static void read_and_refresh_cache(struct replay_opts *opts)
  *     assert(commit_list_count(list) == 2);
  *     return list;
  */
-static struct commit_list **commit_list_append(struct commit *commit,
-					       struct commit_list **next)
+static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
+						struct commit *operand,
+						struct replay_insn_list **next)
 {
-	struct commit_list *new = xmalloc(sizeof(struct commit_list));
-	new->item = commit;
+	struct replay_insn_list *new = xmalloc(sizeof(*new));
+	new->action = action;
+	new->operand = operand;
 	*next = new;
 	new->next = NULL;
 	return &new->next;
 }
 
-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
-		struct replay_opts *opts)
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
 {
-	struct commit_list *cur = NULL;
-	const char *sha1_abbrev = NULL;
-	const char *action_str = opts->action == REVERT ? "revert" : "pick";
-	const char *subject;
-	int subject_len;
+	struct replay_insn_list *cur;
 
 	for (cur = todo_list; cur; cur = cur->next) {
-		sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
-		subject_len = find_commit_subject(cur->item->buffer, &subject);
+		const char *sha1_abbrev, *action_str, *subject;
+		int subject_len;
+
+		action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+		sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+		subject_len = find_commit_subject(cur->operand->buffer, &subject);
 		strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
 			subject_len, subject);
 	}
 	return 0;
 }
 
-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+static int parse_insn_line(char *start, struct replay_insn_list *item)
 {
 	unsigned char commit_sha1[20];
 	char sha1_abbrev[40];
-	enum replay_action action;
 	const char *p, *q;
 
 	p = start;
 	if (!prefixcmp(start, "pick ")) {
-		action = CHERRY_PICK;
+		item->action = REPLAY_PICK;
 		p += strlen("pick ");
 	} else if (!prefixcmp(start, "revert ")) {
-		action = REVERT;
+		item->action = REPLAY_REVERT;
 		p += strlen("revert ");
-	} else
-		return NULL;
+	} else {
+		size_t len = strchrnul(p, '\n') - p;
+		if (len > 255)
+			len = 255;
+		return error(_("Unrecognized action: %.*s"), len, p);
+	}
 
 	q = p + strcspn(p, " \n");
-	if (q - p + 1 > sizeof(sha1_abbrev))
-		return NULL;
+	if (q - p + 1 > sizeof(sha1_abbrev)) {
+		size_t len = q - p;
+		if (len > 255)
+			len = 255;
+		return error(_("Object name too large: %.*s"), len, p);
+	}
 	memcpy(sha1_abbrev, p, q - p);
 	sha1_abbrev[q - p] = '\0';
 
-	/*
-	 * Verify that the action matches up with the one in
-	 * opts; we don't support arbitrary instructions
-	 */
-	if (action != opts->action) {
-		const char *action_str;
-		action_str = action == REVERT ? "revert" : "cherry-pick";
-		error(_("Cannot %s during a %s"), action_str, action_name(opts));
-		return NULL;
-	}
-
 	if (get_sha1(sha1_abbrev, commit_sha1) < 0)
-		return NULL;
+		return error(_("Malformed object name: %s"), sha1_abbrev);
 
-	return lookup_commit_reference(commit_sha1);
+	item->operand = lookup_commit_reference(commit_sha1);
+	if (!item->operand)
+		return error(_("Not a valid commit: %s"), sha1_abbrev);
+
+	item->next = NULL;
+	return 0;
 }
 
-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
-			struct replay_opts *opts)
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
 {
-	struct commit_list **next = todo_list;
-	struct commit *commit;
+	struct replay_insn_list **next = todo_list;
+	struct replay_insn_list item = {0, NULL, NULL};
 	char *p = buf;
 	int i;
 
 	for (i = 1; *p; i++) {
-		commit = parse_insn_line(p, opts);
-		if (!commit)
-			return error(_("Could not parse line %d."), i);
-		next = commit_list_append(commit, next);
+		if (parse_insn_line(p, &item) < 0)
+			return error(_("on line %d."), i);
+		next = replay_insn_list_append(item.action, item.operand, next);
 		p = strchrnul(p, '\n');
 		if (*p)
 			p++;
@@ -749,8 +749,7 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
 	return 0;
 }
 
-static void read_populate_todo(struct commit_list **todo_list,
-			struct replay_opts *opts)
+static void read_populate_todo(struct replay_insn_list **todo_list)
 {
 	const char *todo_file = git_path(SEQ_TODO_FILE);
 	struct strbuf buf = STRBUF_INIT;
@@ -766,7 +765,7 @@ static void read_populate_todo(struct commit_list **todo_list,
 	}
 	close(fd);
 
-	res = parse_insn_buffer(buf.buf, todo_list, opts);
+	res = parse_insn_buffer(buf.buf, todo_list);
 	strbuf_release(&buf);
 	if (res)
 		die(_("Unusable instruction sheet: %s"), todo_file);
@@ -815,18 +814,18 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
 		die(_("Malformed options sheet: %s"), opts_file);
 }
 
-static void walk_revs_populate_todo(struct commit_list **todo_list,
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
 				struct replay_opts *opts)
 {
 	struct rev_info revs;
 	struct commit *commit;
-	struct commit_list **next;
+	struct replay_insn_list **next;
 
 	prepare_revs(&revs, opts);
 
 	next = todo_list;
 	while ((commit = get_revision(&revs)))
-		next = commit_list_append(commit, next);
+		next = replay_insn_list_append(opts->action, commit, next);
 }
 
 static int create_seq_dir(void)
@@ -855,7 +854,7 @@ static void save_head(const char *head)
 		die(_("Error wrapping up %s."), head_file);
 }
 
-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+static void save_todo(struct replay_insn_list *todo_list)
 {
 	const char *todo_file = git_path(SEQ_TODO_FILE);
 	static struct lock_file todo_lock;
@@ -863,7 +862,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
 	int fd;
 
 	fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
-	if (format_todo(&buf, todo_list, opts) < 0)
+	if (format_todo(&buf, todo_list) < 0)
 		die(_("Could not format %s."), todo_file);
 	if (write_in_full(fd, buf.buf, buf.len) < 0) {
 		strbuf_release(&buf);
@@ -907,9 +906,10 @@ static void save_opts(struct replay_opts *opts)
 	}
 }
 
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
+static int pick_commits(struct replay_insn_list *todo_list,
+			struct replay_opts *opts)
 {
-	struct commit_list *cur;
+	struct replay_insn_list *cur;
 	int res;
 
 	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
@@ -919,8 +919,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 	read_and_refresh_cache(opts);
 
 	for (cur = todo_list; cur; cur = cur->next) {
-		save_todo(cur, opts);
-		res = do_pick_commit(cur->item, opts);
+		save_todo(cur);
+		res = do_pick_commit(cur->operand, cur->action, opts);
 		if (res) {
 			if (!cur->next)
 				/*
@@ -945,7 +945,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 
 static int pick_revisions(struct replay_opts *opts)
 {
-	struct commit_list *todo_list = NULL;
+	struct replay_insn_list *todo_list = NULL;
 	unsigned char sha1[20];
 
 	read_and_refresh_cache(opts);
@@ -962,7 +962,7 @@ static int pick_revisions(struct replay_opts *opts)
 		if (!file_exists(git_path(SEQ_TODO_FILE)))
 			goto error;
 		read_populate_opts(&opts);
-		read_populate_todo(&todo_list, opts);
+		read_populate_todo(&todo_list);
 
 		/* Verify that the conflict has been resolved */
 		if (!index_differs_from("HEAD", 0))
@@ -982,7 +982,7 @@ static int pick_revisions(struct replay_opts *opts)
 			return -1;
 		}
 		if (get_sha1("HEAD", sha1)) {
-			if (opts->action == REVERT)
+			if (opts->action == REPLAY_REVERT)
 				return error(_("Can't revert as initial commit"));
 			return error(_("Can't cherry-pick into empty head"));
 		}
@@ -1002,7 +1002,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	memset(&opts, 0, sizeof(opts));
 	if (isatty(0))
 		opts.edit = 1;
-	opts.action = REVERT;
+	opts.action = REPLAY_REVERT;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
 	res = pick_revisions(&opts);
@@ -1017,7 +1017,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 	int res;
 
 	memset(&opts, 0, sizeof(opts));
-	opts.action = CHERRY_PICK;
+	opts.action = REPLAY_PICK;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
 	res = pick_revisions(&opts);
diff --git a/sequencer.h b/sequencer.h
index 905d295..f4db257 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -7,6 +7,14 @@
 #define SEQ_TODO_FILE	"sequencer/todo"
 #define SEQ_OPTS_FILE	"sequencer/opts"
 
+enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+
+struct replay_insn_list {
+	enum replay_action action;
+	struct commit *operand;
+	struct replay_insn_list *next;
+};
+
 /*
  * Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
  * any errors.  Intended to be used by 'git reset'.
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 39b55c1..4b12244 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -239,4 +239,62 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
 	test_line_count = 4 commits
 '
 
+test_expect_success 'revert --continue continues after cherry-pick' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	git revert --continue &&
+	test_path_is_missing .git/sequencer &&
+	{
+		git rev-list HEAD |
+		git diff-tree --root --stdin |
+		sed "s/$_x40/OBJID/g"
+	} >actual &&
+	cat >expect <<-\EOF &&
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:000000 100644 OBJID OBJID A	foo
+	:000000 100644 OBJID OBJID A	unrelated
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'mixed pick and revert instructions' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	oldsha=`git rev-parse --short HEAD~1` &&
+	echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
+	git cherry-pick --continue &&
+	test_path_is_missing .git/sequencer &&
+	{
+		git rev-list HEAD |
+		git diff-tree --root --stdin |
+		sed "s/$_x40/OBJID/g"
+	} >actual &&
+	cat >expect <<-\EOF &&
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:000000 100644 OBJID OBJID A	foo
+	:000000 100644 OBJID OBJID A	unrelated
+	EOF
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 4/6] revert: Make commit descriptions in insn sheet optional
From: Ramkumar Ramachandra @ 2011-10-08 17:36 UTC (permalink / raw)
  To: Git List
  Cc: Jonathan Nieder, Junio C Hamano, Jeff King, Daniel Barkalow,
	Christian Couder
In-Reply-To: <1318095407-26429-1-git-send-email-artagnon@gmail.com>

Change the instruction sheet format subtly so that a description of
the commit after the object name is optional.  As a result, an
instruction sheet like this is now perfectly valid:

  pick 35b0426
  pick fbd5bbcbc2e
  pick 7362160f

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c                |   19 ++++++++-----------
 t/t3510-cherry-pick-sequence.sh |   14 ++++++++++++++
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 6451089..aa6c34e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -692,26 +692,23 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
 	unsigned char commit_sha1[20];
 	char sha1_abbrev[40];
 	enum replay_action action;
-	int insn_len = 0;
-	char *p, *q;
+	const char *p, *q;
 
+	p = start;
 	if (!prefixcmp(start, "pick ")) {
 		action = CHERRY_PICK;
-		insn_len = strlen("pick");
-		p = start + insn_len + 1;
+		p += strlen("pick ");
 	} else if (!prefixcmp(start, "revert ")) {
 		action = REVERT;
-		insn_len = strlen("revert");
-		p = start + insn_len + 1;
+		p += strlen("revert ");
 	} else
 		return NULL;
 
-	q = strchr(p, ' ');
-	if (!q || q - p + 1 > sizeof(sha1_abbrev))
+	q = p + strcspn(p, " \n");
+	if (q - p + 1 > sizeof(sha1_abbrev))
 		return NULL;
-	q++;
-
-	strlcpy(sha1_abbrev, p, q - p);
+	memcpy(sha1_abbrev, p, q - p);
+	sha1_abbrev[q - p] = '\0';
 
 	/*
 	 * Verify that the action matches up with the one in
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 2113308..39b55c1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -225,4 +225,18 @@ test_expect_success 'malformed instruction sheet 3' '
 	test_must_fail git cherry-pick --continue
 '
 
+test_expect_success 'commit descriptions in insn sheet are optional' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+	cp new_sheet .git/sequencer/todo &&
+	git cherry-pick --continue &&
+	test_path_is_missing .git/sequencer &&
+	git rev-list HEAD >commits
+	test_line_count = 4 commits
+'
+
 test_done
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 3/6] revert: Fix buffer overflow in insn sheet parser
From: Ramkumar Ramachandra @ 2011-10-08 17:36 UTC (permalink / raw)
  To: Git List
  Cc: Jonathan Nieder, Junio C Hamano, Jeff King, Daniel Barkalow,
	Christian Couder
In-Reply-To: <1318095407-26429-1-git-send-email-artagnon@gmail.com>

Check that the commit name argument to a "pick" or "revert" action in
'.git/sequencer/todo' is not too long, to avoid overflowing an
on-stack buffer.  This fixes a regression introduced by 5a5d80f4
(revert: Introduce --continue to continue the operation, 2011-08-04).

Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c                |    2 +-
 t/t3510-cherry-pick-sequence.sh |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index b3c5e0e..6451089 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -707,7 +707,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
 		return NULL;
 
 	q = strchr(p, ' ');
-	if (!q)
+	if (!q || q - p + 1 > sizeof(sha1_abbrev))
 		return NULL;
 	q++;
 
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3bca2b3..2113308 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -12,6 +12,9 @@ test_description='Test cherry-pick continuation features
 
 . ./test-lib.sh
 
+# Repeat first match 10 times
+_r10='\1\1\1\1\1\1\1\1\1\1'
+
 pristine_detach () {
 	git cherry-pick --reset &&
 	git checkout -f "$1^0" &&
@@ -211,4 +214,15 @@ test_expect_success 'malformed instruction sheet 2' '
 	test_must_fail git cherry-pick --continue
 '
 
+test_expect_success 'malformed instruction sheet 3' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "resolved" >foo &&
+	git add foo &&
+	git commit &&
+	sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
+	cp new_sheet .git/sequencer/todo &&
+	test_must_fail git cherry-pick --continue
+'
+
 test_done
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 2/6] revert: Simplify getting commit subject
From: Ramkumar Ramachandra @ 2011-10-08 17:36 UTC (permalink / raw)
  To: Git List
  Cc: Jonathan Nieder, Junio C Hamano, Jeff King, Daniel Barkalow,
	Christian Couder
In-Reply-To: <1318095407-26429-1-git-send-email-artagnon@gmail.com>

The heavy parsing and memory allocations performed by get_message is
unnecessary when only the commit subject is desired.  Use
find_commit_subject instead.

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index a2c304d..b3c5e0e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -673,16 +673,16 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
 		struct replay_opts *opts)
 {
 	struct commit_list *cur = NULL;
-	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
 	const char *sha1_abbrev = NULL;
 	const char *action_str = opts->action == REVERT ? "revert" : "pick";
+	const char *subject;
+	int subject_len;
 
 	for (cur = todo_list; cur; cur = cur->next) {
 		sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
-		if (get_message(cur->item, &msg))
-			return error(_("Cannot get commit message for %s"), sha1_abbrev);
-		strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
-		free_message(&msg);
+		subject_len = find_commit_subject(cur->item->buffer, &subject);
+		strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
+			subject_len, subject);
 	}
 	return 0;
 }
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 1/6] revert: Free memory after get_message call
From: Ramkumar Ramachandra @ 2011-10-08 17:36 UTC (permalink / raw)
  To: Git List
  Cc: Jonathan Nieder, Junio C Hamano, Jeff King, Daniel Barkalow,
	Christian Couder
In-Reply-To: <1318095407-26429-1-git-send-email-artagnon@gmail.com>

The format_todo function leaks memory because it forgets to call
free_message after get_message.  It is a potentially big leak, because
fresh memory is allocated to store the commit message message for each
commit.  Fix this.

Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index ba27cf1..a2c304d 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -682,6 +682,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
 		if (get_message(cur->item, &msg))
 			return error(_("Cannot get commit message for %s"), sha1_abbrev);
 		strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
+		free_message(&msg);
 	}
 	return 0;
 }
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 0/6] Sequencer fixups mini-series
From: Ramkumar Ramachandra @ 2011-10-08 17:36 UTC (permalink / raw)
  To: Git List
  Cc: Jonathan Nieder, Junio C Hamano, Jeff King, Daniel Barkalow,
	Christian Couder

Hi,

Now that the original sequencer series has hit 'master' (cd4093b6), we
can now build on it!  Unfortunately, as outlined in $gmane/179613,
there are several UI design difficulties that we need to surmount.  As
a prelude, I've decided to prepare this mini-series for fixing up a
few minor issues before attacking the problem; please see
$gmane/179304 for relevant discussions.

The differences are:
1. I've dropped the last two parts in the previous iteration.
2. Part 2 is new.  Thanks to Jonathan for the suggestion.
3. Minor fixups and commit message improvements in response to
reviews.

p.s- I'm travelling this week, and won't be able to respond to
reviews until the 16th.

Thanks.

-- Ram

Jonathan Nieder (1):
  revert: Simplify passing command-line arguments around

Ramkumar Ramachandra (5):
  revert: Free memory after get_message call
  revert: Simplify getting commit subject
  revert: Fix buffer overflow in insn sheet parser
  revert: Make commit descriptions in insn sheet optional
  revert: Allow mixed pick and revert instructions

 builtin/revert.c                |  209 ++++++++++++++++++++-------------------
 sequencer.h                     |    8 ++
 t/t3510-cherry-pick-sequence.sh |   86 ++++++++++++++++
 3 files changed, 200 insertions(+), 103 deletions(-)

-- 
1.7.4.1

^ permalink raw reply

* [PATCH] builtin/log.c: Fix an "Using plain integer as NULL pointer" warning
From: Ramsay Jones @ 2011-10-08 16:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT Mailing-list


Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Junio,

I wanted to request that you squash this into commit 739453a3
(format-patch: use branch description in cover letter, 21-09-2011).
But, since that's in next ...

Hmm, I should probably start at least building the pu branch so that
I could catch these things earlier ...

ATB,
Ramsay Jones

 builtin/log.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index e80a925..4395f3e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1040,7 +1040,7 @@ static char *find_branch_name(struct rev_info *rev)
 	if (positive < 0)
 		return NULL;
 	strbuf_addf(&buf, "refs/heads/%s", rev->cmdline.rev[positive].name);
-	branch = resolve_ref(buf.buf, branch_sha1, 1, 0);
+	branch = resolve_ref(buf.buf, branch_sha1, 1, NULL);
 	if (!branch ||
 	    prefixcmp(branch, "refs/heads/") ||
 	    hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1))
-- 
1.7.7

^ permalink raw reply related

* Re: [PATCH 2/3] Fix some "variable might be used uninitialized" warnings
From: Ramsay Jones @ 2011-10-08 16:06 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <4E6FDBA4.6050505@ramsay1.demon.co.uk>

Ramsay Jones wrote:
> Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>
>>> In particular, gcc complains as follows:
>>>
>>>         CC tree-walk.o
>>>     tree-walk.c: In function `traverse_trees':
>>>     tree-walk.c:347: warning: 'e' might be used uninitialized in this \
>>>         function
>>>
>>>         CC builtin/revert.o
>>>     builtin/revert.c: In function `verify_opt_mutually_compatible':
>>>     builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
>>>         this function
>> Could you also add something to this effect to the commit log message:
>>
>> 	but I have verified that these are gcc being not careful
>> 	enough and they are never used uninitialized.
> 
> see below for the v2 patch.
> 
>> If that is what you indeed have done, that is.
> 
> Indeed. The builtin/revert.c warning is straight-forward, but the tree-walk.c
> warning is somewhat less so! ;-)
> 
> Imagine traverse_trees() (tree-walk.c:324) was called with n == 0 (let's ignore
> the effective calls to xmalloc(0) and xcalloc(0,..) at the start of that function).
> At first blush it looked like 'e' would remain uninitialized in the call to
> prune_traversal() at line 403.  Indeed it *would* be if you ever got to that line.
> However, since the 'mask' variable (set at line 391) remains set to zero at line 401,
> the flow of control leaves the loop before 'e' is used.
> 
> [I don't think traverse_trees() would ever be called with n == 0 anyway; the call
> site in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)
> which start from unpack_trees() are protected by "if (len)", where 'len' is unsigned.]

When patches don't even make it to pu I just assume you hate them so much that
there is not much chance of them being applied and simply forget about them.
In this case, since compiler warnings are a bugbear of mine, I'm hoping that
you just forgot about this one ... :-D  [if not, sorry for the noise].

For your convenience, I've attached the patch below (rebased against current master).

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] Fix some "variable might be used uninitialized" warnings

In particular, gcc complains as follows:

        CC tree-walk.o
    tree-walk.c: In function `traverse_trees':
    tree-walk.c:347: warning: 'e' might be used uninitialized in this \
        function

        CC builtin/revert.o
    builtin/revert.c: In function `verify_opt_mutually_compatible':
    builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
        this function

However, I have verified that the analysis performed by gcc was too
conservative and that these variables are not, in fact, used while
uninitialized.

In order to suppress the warnings, we add an NULL pointer initializer
to the declarations of the 'e' and 'opt2' variables.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 builtin/revert.c |    2 +-
 tree-walk.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index ba27cf1..200149e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -110,7 +110,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
 
 static void verify_opt_mutually_compatible(const char *me, ...)
 {
-	const char *opt1, *opt2;
+	const char *opt1, *opt2 = NULL;
 	va_list ap;
 
 	va_start(ap, me);
diff --git a/tree-walk.c b/tree-walk.c
index 808bb55..a8d8a66 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -344,7 +344,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 		unsigned long mask, dirmask;
 		const char *first = NULL;
 		int first_len = 0;
-		struct name_entry *e;
+		struct name_entry *e = NULL;
 		int len;
 
 		for (i = 0; i < n; i++) {
-- 
1.7.7

^ permalink raw reply related

* Re: How pretty is pretty? git cat-file -p inconsistency
From: Jakub Narebski @ 2011-10-08 16:36 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, Git Mailing List, Linus Torvalds
In-Reply-To: <4E906292.1020909@drmicha.warpmail.net>

On Sat, 8 Oct 2011, Michael J Gruber wrote:
> Jakub Narebski venit, vidit, dixit 08.10.2011 01:50:

> > Tree object consist of zero or more entries.  Each item consist of mode,
> > filename, and sha1:
> > 
> >   <mode> SPC <filename> NUL <sha1>
> > 
> > where
> > 
> > 1. <mode> is variable-length (!) text (!) containing mode of an
> >    entry. It encodes type of entry: if it is blob (including special
> >    case: symbolic link), tree i.e. directory, or a commit
> >    i.e. submodule.  Does not include leading zeros.
> > 
> > 2. <filename> is variable-length null-terminated ("\0") name of a file
> >    or directory, or name of directory where submodule is attached
> > 
> > 3. <sha1> is 40-bytes _binary_ identifier.
> > 
> > HTH
> 
> It does help, thanks.
> 
> Though I'm beginning to think we have a crazy object format. Not only do
> we have a lot of indirections (like ascii representation of decimal
> representation of length), but we store sha1 as ascii in commit and tag
> objects and as binary in tree objects. Which makes tree objects the only
> unpleasant ones to look at (and parse) in raw form. (I was hoping we can
> dispose of/deprecate cat-file -p in favor of show). Oh well.

Well, actually we have pretty consistent format, i.e. we use textual
format everywhere (textual size of blob instead of some variable-length
integer, textual name of type of object instead of a byte for it, epoch
as a text and not 64bit signed int in some network format, hexadecimal sha1,
space separated (sub)fields)... 

... with the sole exception of tree object, which uses _binary_ sha1.
What was Linus thinking?!?

To have consistency the tree entry should IMVHO look like this

  <textual mode> SPC <filename> NUL <hexadecimal sha1> LF


Nb. with hexadecimal sha-1 everywhere it would be I think possible (if very
very difficult) to move to different hash function: SHA-256, Skein, etc.
I don't know if it is now at all possible...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH 9/9] completion: remove broken dead code from __git_heads() and __git_tags()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

__git_heads() was introduced in 5de40f5 (Teach bash about
git-repo-config., 2006-11-27), and __git_tags() in 88e21dc (Teach bash
about completing arguments for git-tag, 2007-08-31).  As their name
suggests, __git_heads() is supposed to list only branches, and
__git_tags() only tags.

Since their introduction both of these functions consist of two
distinct parts.  The first part gets branches or tags, respectively,
from a local repositoty using 'git for-each-ref'.  The second part
queries a remote repository given as argument using 'git ls-remote'.

These remote-querying parts are broken in both functions since their
introduction, because they list both branches and tags from the remote
repository.  (The 'git ls-remote' query is not limited to list only
heads or tags, respectively, and the for loop filtering the query
results prints everything except dereferenced tags.)  This breakage
could be easily fixed by passing the '--heads' or '--tags' options or
appropriate refs patterns to the 'git ls-remote' invocations.

However, that no one noticed this breakage yet is probably not a
coincidence: neither of these two functions were used to query a
remote repository, the remote-querying parts were dead code already
upon thier introduction and remained dead ever since.

Since those parts of code are broken, are and were never used, stop
the bit-rotting and remove them.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   22 ++--------------------
 1 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d7151220..802b703d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -536,42 +536,24 @@ __gitcomp_nl ()
 	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
 }
 
-# __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
 __git_heads ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
+	local dir="$(__gitdir)"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/heads
 		return
 	fi
-	for i in $(git ls-remote "${1-}" 2>/dev/null); do
-		case "$is_hash,$i" in
-		y,*) is_hash=n ;;
-		n,*^{}) is_hash=y ;;
-		n,refs/heads/*) is_hash=y; echo "${i#refs/heads/}" ;;
-		n,*) is_hash=y; echo "$i" ;;
-		esac
-	done
 }
 
-# __git_tags accepts 0 or 1 arguments (to pass to __gitdir)
 __git_tags ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
+	local dir="$(__gitdir)"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/tags
 		return
 	fi
-	for i in $(git ls-remote "${1-}" 2>/dev/null); do
-		case "$is_hash,$i" in
-		y,*) is_hash=n ;;
-		n,*^{}) is_hash=y ;;
-		n,refs/tags/*) is_hash=y; echo "${i#refs/tags/}" ;;
-		n,*) is_hash=y; echo "$i" ;;
-		esac
-	done
 }
 
 # __git_refs accepts 0, 1 (to pass to __gitdir), or 2 arguments
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 8/9] completion: fast initial completion for config 'remote.*.fetch' value
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

Refspecs for branches in a remote repository start with 'refs/heads/',
so completing those refspecs with 'git config remote.origin.fetch
<TAB>' always offers 'refs/heads/' first, because that's the unique
part of the possible refspecs.  But it does so only after querying the
remote with 'git ls-remote', which can take a while when the request
goes through some slower network to a remote server.

Don't waste the user's time and offer 'refs/heads/' right away for
'git config remote.origin.fetch <TAB>'.

The reason for putting 'refs/heads/' directly into COMPREPLY instead
of using __gitcomp() is to avoid __gitcomp() adding a trailing space.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 658df3a7..d7151220 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1890,6 +1890,10 @@ _git_config ()
 	remote.*.fetch)
 		local remote="${prev#remote.}"
 		remote="${remote%.fetch}"
+		if [ -z "$cur" ]; then
+			COMPREPLY=("refs/heads/")
+			return
+		fi
 		__gitcomp_nl "$(__git_refs_remotes "$remote")"
 		return
 		;;
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 7/9] completion: improve ls-remote output filtering in __git_refs_remotes()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

This follows suit of a previous patch for __git_refs(): use a
while-read loop and let bash's word splitting get rid of object names
from 'git ls-remote's output.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dc1d5e90..658df3a7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -650,15 +650,10 @@ __git_refs2 ()
 # __git_refs_remotes requires 1 argument (to pass to ls-remote)
 __git_refs_remotes ()
 {
-	local i is_hash=y
-	for i in $(git ls-remote "$1" 'refs/heads/*' 2>/dev/null); do
-		case "$is_hash" in
-		n)
-			is_hash=y
-			echo "$i:refs/remotes/$1/${i#refs/heads/}"
-			;;
-		y) is_hash=n ;;
-		esac
+	local i hash
+	git ls-remote "$1" 'refs/heads/*' 2>/dev/null | \
+	while read hash i; do
+		echo "$i:refs/remotes/$1/${i#refs/heads/}"
 	done
 }
 
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 6/9] completion: query only refs/heads/ in __git_refs_remotes()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

__git_refs_remotes() is used to provide completion for refspecs to set
'remote.*.fetch' config variables for branches on the given remote.
So it's really only interested in refs under 'refs/heads/', but it
queries the remote for all its refs and then filters out all refs
outside of 'refs/heads/'.

Let 'git ls-remote' do the filtering.

Also remove the unused $cmd variable from __git_refs_remotes().

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8d3597e..dc1d5e90 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -650,17 +650,14 @@ __git_refs2 ()
 # __git_refs_remotes requires 1 argument (to pass to ls-remote)
 __git_refs_remotes ()
 {
-	local cmd i is_hash=y
-	for i in $(git ls-remote "$1" 2>/dev/null); do
-		case "$is_hash,$i" in
-		n,refs/heads/*)
+	local i is_hash=y
+	for i in $(git ls-remote "$1" 'refs/heads/*' 2>/dev/null); do
+		case "$is_hash" in
+		n)
 			is_hash=y
 			echo "$i:refs/remotes/$1/${i#refs/heads/}"
 			;;
-		y,*) is_hash=n ;;
-		n,*^{}) is_hash=y ;;
-		n,refs/tags/*) is_hash=y;;
-		n,*) is_hash=y; ;;
+		y) is_hash=n ;;
 		esac
 	done
 }
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 5/9] completion: support full refs from remote repositories
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

When the __git_refs() completion helper function lists refs from a
local repository, it usually lists the refs' short name, except when
it needs to provide completion for words starting with refs, because
in that case it lists full ref names, see 608efb87 (bash: complete
full refs, 2008-11-28).

Add the same functionality to the code path dealing with remote
repositories, too.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   29 +++++++++++++++++++++--------
 1 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c6ab742d..a8d3597e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -615,14 +615,27 @@ __git_refs ()
 		fi
 		return
 	fi
-	git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
-	while read hash i; do
-		case "$i" in
-		*^{}) ;;
-		refs/*) echo "${i#refs/*/}" ;;
-		*) echo "$i" ;;
-		esac
-	done
+	case "$cur" in
+	refs|refs/*)
+		git ls-remote "$dir" "$cur*" 2>/dev/null | \
+		while read hash i; do
+			case "$i" in
+			*^{}) ;;
+			*) echo "$i" ;;
+			esac
+		done
+		;;
+	*)
+		git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
+		while read hash i; do
+			case "$i" in
+			*^{}) ;;
+			refs/*) echo "${i#refs/*/}" ;;
+			*) echo "$i" ;;
+			esac
+		done
+		;;
+	esac
 }
 
 # __git_refs2 requires 1 argument (to pass to __git_refs)
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 4/9] completion: improve ls-remote output filtering in __git_refs()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

The remote-handling part of __git_refs() has a nice for loop and state
machine case statement to iterate over all words from the output of
'git ls-remote' to identify object names and ref names.  Since each
line in the output of 'git ls-remote' consists of an object name and a
ref name, we can do more effective filtering by using a while-read
loop and letting bash's word splitting take care of object names.
This way the code is easier to understand and the loop will need only
half the number of iterations than before.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6b5dc5cd..c6ab742d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -579,7 +579,7 @@ __git_tags ()
 # by checkout for tracking branches
 __git_refs ()
 {
-	local i is_hash=y dir="$(__gitdir "${1-}")" track="${2-}"
+	local i hash dir="$(__gitdir "${1-}")" track="${2-}"
 	local format refs
 	if [ -d "$dir" ]; then
 		case "$cur" in
@@ -615,12 +615,12 @@ __git_refs ()
 		fi
 		return
 	fi
-	for i in $(git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null); do
-		case "$is_hash,$i" in
-		y,*) is_hash=n ;;
-		n,*^{}) is_hash=y ;;
-		n,refs/*) is_hash=y; echo "${i#refs/*/}" ;;
-		n,*) is_hash=y; echo "$i" ;;
+	git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
+	while read hash i; do
+		case "$i" in
+		*^{}) ;;
+		refs/*) echo "${i#refs/*/}" ;;
+		*) echo "$i" ;;
 		esac
 	done
 }
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 3/9] completion: make refs completion consistent for local and remote repos
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

For a local repository the __git_refs() completion helper function
lists refs under 'refs/(tags|heads|remotes)/', plus some special refs
like HEAD and ORIG_HEAD.  For a remote repository, however, it lists
all refs.

Fix this inconsistency by specifying refs filter patterns for 'git
ls-remote' to only list refs under 'refs/(tags|heads|remotes)/'.

For now this makes it impossible to complete refs outside of
'refs/(tags|heads|remotes)/' in a remote repository, but a followup
patch will resurrect that.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 86de0bf4..6b5dc5cd 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -615,13 +615,11 @@ __git_refs ()
 		fi
 		return
 	fi
-	for i in $(git ls-remote "$dir" 2>/dev/null); do
+	for i in $(git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null); do
 		case "$is_hash,$i" in
 		y,*) is_hash=n ;;
 		n,*^{}) is_hash=y ;;
-		n,refs/tags/*) is_hash=y; echo "${i#refs/tags/}" ;;
-		n,refs/heads/*) is_hash=y; echo "${i#refs/heads/}" ;;
-		n,refs/remotes/*) is_hash=y; echo "${i#refs/remotes/}" ;;
+		n,refs/*) is_hash=y; echo "${i#refs/*/}" ;;
 		n,*) is_hash=y; echo "$i" ;;
 		esac
 	done
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 2/9] completion: optimize refs completion
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

After a unique command or option is completed, in most cases it is a
good thing to add a trailing a space, but sometimes it doesn't makes
sense, e.g. when the completed word is an option taking an argument
('--option=') or a configuration section ('core.').  Therefore the
completion script uses the '-o nospace' option to prevent bash from
automatically appending a space to unique completions, and it has the
__gitcomp() function to add that trailing space only when necessary.
See 72e5e989 (bash: Add space after unique command name is completed.,
2007-02-04), 78d4d6a2 (bash: Support unique completion on git-config.,
2007-02-04), and b3391775 (bash: Support unique completion when
possible., 2007-02-04).

__gitcomp() therefore iterates over all possible completion words it
got as argument, and checks each word whether a trailing space is
necessary or not.  This is ok for commands, options, etc., i.e. when
the number of words is relatively small, but can be noticeably slow
for large number of refs.  However, while options might or might not
need that trailing space, refs are always handled uniformly and always
get that trailing space (or a trailing '.' for 'git config
branch.<head>.').  Since refs listed by __git_refs() & co. are
separated by newline, this allows us some optimizations with
'compgen'.

So, add a specialized variant of __gitcomp() that only deals with
possible completion words separated by a newline and uniformly appends
the trailing space to all words using 'compgen -S' (or any other
suffix, if specified), so no iteration over all words is done.
Convert all callsites of __gitcomp() where it's called with refs, i.e.
when it gets the output of either __git_refs(), __git_heads(),
__git_tags(), __git_refs2(), __git_refs_remotes(), or the odd 'git
for-each-ref' somewhere in _git_config().  Also convert callsites
where it gets other uniformly handled newline separated word lists,
i.e. either remotes from __git_remotes(), names of set configuration
variables from __git_config_get_set_variables(), stashes, or commands
and aliases.

Here are some timing results for dealing with 10000 refs.
Before:

  $ refs="$(__git_refs ~/tmp/git/repo-with-10k-refs/)"
  $ time __gitcomp "$refs"

  real	0m1.134s
  user	0m1.060s
  sys	0m0.130s

After:

  $ time __gitcomp_nl "$refs"

  real	0m0.373s
  user	0m0.360s
  sys	0m0.020s

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |  116 +++++++++++++++++++-------------
 1 files changed, 70 insertions(+), 46 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c0fb6e15..86de0bf4 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -512,6 +512,30 @@ __gitcomp ()
 	esac
 }
 
+# Generates completion reply with compgen.
+# It accepts 1 to 4 arguments:
+# 1: List of possible completion words, separated by a single newline.
+# 2: A prefix to be added to each possible completion word (optional).
+# 3: Generate possible completion matches for this word (optional).
+# 4: A suffix to be appended to each possible completion word (optional).
+#    If omitted, a space is appended; if specified but empty, nothing is
+#    appended.
+__gitcomp_nl ()
+{
+	local s=$'\n' IFS=' '$'\t'$'\n'
+	local cur_="$cur" suffix=" "
+
+	if [ $# -gt 2 ]; then
+		cur_="$3"
+		if [ $# -gt 3 ]; then
+			suffix="$4"
+		fi
+	fi
+
+	IFS=$s
+	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
+}
+
 # __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
 __git_heads ()
 {
@@ -716,15 +740,15 @@ __git_complete_revlist_file ()
 	*...*)
 		pfx="${cur_%...*}..."
 		cur_="${cur_#*...}"
-		__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+		__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		;;
 	*..*)
 		pfx="${cur_%..*}.."
 		cur_="${cur_#*..}"
-		__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+		__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		;;
 	*)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		;;
 	esac
 }
@@ -764,7 +788,7 @@ __git_complete_remote_or_refspec ()
 		c=$((++c))
 	done
 	if [ -z "$remote" ]; then
-		__gitcomp "$(__git_remotes)"
+		__gitcomp_nl "$(__git_remotes)"
 		return
 	fi
 	if [ $no_complete_refspec = 1 ]; then
@@ -789,23 +813,23 @@ __git_complete_remote_or_refspec ()
 	case "$cmd" in
 	fetch)
 		if [ $lhs = 1 ]; then
-			__gitcomp "$(__git_refs2 "$remote")" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs2 "$remote")" "$pfx" "$cur_"
 		else
-			__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		fi
 		;;
 	pull)
 		if [ $lhs = 1 ]; then
-			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
 		else
-			__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		fi
 		;;
 	push)
 		if [ $lhs = 1 ]; then
-			__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		else
-			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
 		fi
 		;;
 	esac
@@ -1084,7 +1108,7 @@ _git_archive ()
 		return
 		;;
 	--remote=*)
-		__gitcomp "$(__git_remotes)" "" "${cur##--remote=}"
+		__gitcomp_nl "$(__git_remotes)" "" "${cur##--remote=}"
 		return
 		;;
 	--*)
@@ -1115,7 +1139,7 @@ _git_bisect ()
 
 	case "$subcommand" in
 	bad|good|reset|skip|start)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		;;
 	*)
 		COMPREPLY=()
@@ -1146,9 +1170,9 @@ _git_branch ()
 		;;
 	*)
 		if [ $only_local_ref = "y" -a $has_r = "n" ]; then
-			__gitcomp "$(__git_heads)"
+			__gitcomp_nl "$(__git_heads)"
 		else
-			__gitcomp "$(__git_refs)"
+			__gitcomp_nl "$(__git_refs)"
 		fi
 		;;
 	esac
@@ -1195,7 +1219,7 @@ _git_checkout ()
 		if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
 			track=''
 		fi
-		__gitcomp "$(__git_refs '' $track)"
+		__gitcomp_nl "$(__git_refs '' $track)"
 		;;
 	esac
 }
@@ -1212,7 +1236,7 @@ _git_cherry_pick ()
 		__gitcomp "--edit --no-commit"
 		;;
 	*)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		;;
 	esac
 }
@@ -1266,7 +1290,7 @@ _git_commit ()
 		;;
 	--reuse-message=*|--reedit-message=*|\
 	--fixup=*|--squash=*)
-		__gitcomp "$(__git_refs)" "" "${cur#*=}"
+		__gitcomp_nl "$(__git_refs)" "" "${cur#*=}"
 		return
 		;;
 	--untracked-files=*)
@@ -1297,7 +1321,7 @@ _git_describe ()
 			"
 		return
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 __git_diff_common_options="--stat --numstat --shortstat --summary
@@ -1456,7 +1480,7 @@ _git_grep ()
 		;;
 	esac
 
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_help ()
@@ -1514,7 +1538,7 @@ _git_ls_files ()
 
 _git_ls_remote ()
 {
-	__gitcomp "$(__git_remotes)"
+	__gitcomp_nl "$(__git_remotes)"
 }
 
 _git_ls_tree ()
@@ -1610,7 +1634,7 @@ _git_merge ()
 		__gitcomp "$__git_merge_options"
 		return
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_mergetool ()
@@ -1630,7 +1654,7 @@ _git_mergetool ()
 
 _git_merge_base ()
 {
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_mv ()
@@ -1661,7 +1685,7 @@ _git_notes ()
 	,*)
 		case "${words[cword-1]}" in
 		--ref)
-			__gitcomp "$(__git_refs)"
+			__gitcomp_nl "$(__git_refs)"
 			;;
 		*)
 			__gitcomp "$subcommands --ref"
@@ -1670,7 +1694,7 @@ _git_notes ()
 		;;
 	add,--reuse-message=*|append,--reuse-message=*|\
 	add,--reedit-message=*|append,--reedit-message=*)
-		__gitcomp "$(__git_refs)" "" "${cur#*=}"
+		__gitcomp_nl "$(__git_refs)" "" "${cur#*=}"
 		;;
 	add,--*|append,--*)
 		__gitcomp '--file= --message= --reedit-message=
@@ -1689,7 +1713,7 @@ _git_notes ()
 		-m|-F)
 			;;
 		*)
-			__gitcomp "$(__git_refs)"
+			__gitcomp_nl "$(__git_refs)"
 			;;
 		esac
 		;;
@@ -1717,12 +1741,12 @@ _git_push ()
 {
 	case "$prev" in
 	--repo)
-		__gitcomp "$(__git_remotes)"
+		__gitcomp_nl "$(__git_remotes)"
 		return
 	esac
 	case "$cur" in
 	--repo=*)
-		__gitcomp "$(__git_remotes)" "" "${cur##--repo=}"
+		__gitcomp_nl "$(__git_remotes)" "" "${cur##--repo=}"
 		return
 		;;
 	--*)
@@ -1760,7 +1784,7 @@ _git_rebase ()
 
 		return
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_reflog ()
@@ -1771,7 +1795,7 @@ _git_reflog ()
 	if [ -z "$subcommand" ]; then
 		__gitcomp "$subcommands"
 	else
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 	fi
 }
 
@@ -1853,23 +1877,23 @@ _git_config ()
 {
 	case "$prev" in
 	branch.*.remote)
-		__gitcomp "$(__git_remotes)"
+		__gitcomp_nl "$(__git_remotes)"
 		return
 		;;
 	branch.*.merge)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		return
 		;;
 	remote.*.fetch)
 		local remote="${prev#remote.}"
 		remote="${remote%.fetch}"
-		__gitcomp "$(__git_refs_remotes "$remote")"
+		__gitcomp_nl "$(__git_refs_remotes "$remote")"
 		return
 		;;
 	remote.*.push)
 		local remote="${prev#remote.}"
 		remote="${remote%.push}"
-		__gitcomp "$(git --git-dir="$(__gitdir)" \
+		__gitcomp_nl "$(git --git-dir="$(__gitdir)" \
 			for-each-ref --format='%(refname):%(refname)' \
 			refs/heads)"
 		return
@@ -1916,7 +1940,7 @@ _git_config ()
 		return
 		;;
 	--get|--get-all|--unset|--unset-all)
-		__gitcomp "$(__git_config_get_set_variables)"
+		__gitcomp_nl "$(__git_config_get_set_variables)"
 		return
 		;;
 	*.*)
@@ -1942,7 +1966,7 @@ _git_config ()
 		;;
 	branch.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
-		__gitcomp "$(__git_heads)" "$pfx" "$cur_" "."
+		__gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
 		return
 		;;
 	guitool.*.*)
@@ -1971,7 +1995,7 @@ _git_config ()
 	pager.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
 		__git_compute_all_commands
-		__gitcomp "$__git_all_commands" "$pfx" "$cur_"
+		__gitcomp_nl "$__git_all_commands" "$pfx" "$cur_"
 		return
 		;;
 	remote.*.*)
@@ -1984,7 +2008,7 @@ _git_config ()
 		;;
 	remote.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
-		__gitcomp "$(__git_remotes)" "$pfx" "$cur_" "."
+		__gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
 		return
 		;;
 	url.*.*)
@@ -2285,7 +2309,7 @@ _git_remote ()
 
 	case "$subcommand" in
 	rename|rm|show|prune)
-		__gitcomp "$(__git_remotes)"
+		__gitcomp_nl "$(__git_remotes)"
 		;;
 	update)
 		local i c='' IFS=$'\n'
@@ -2303,7 +2327,7 @@ _git_remote ()
 
 _git_replace ()
 {
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_reset ()
@@ -2316,7 +2340,7 @@ _git_reset ()
 		return
 		;;
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_revert ()
@@ -2327,7 +2351,7 @@ _git_revert ()
 		return
 		;;
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_rm ()
@@ -2426,7 +2450,7 @@ _git_stash ()
 			COMPREPLY=()
 			;;
 		show,*|apply,*|drop,*|pop,*|branch,*)
-			__gitcomp "$(git --git-dir="$(__gitdir)" stash list \
+			__gitcomp_nl "$(git --git-dir="$(__gitdir)" stash list \
 					| sed -n -e 's/:.*//p')"
 			;;
 		*)
@@ -2560,7 +2584,7 @@ _git_tag ()
 		i="${words[c]}"
 		case "$i" in
 		-d|-v)
-			__gitcomp "$(__git_tags)"
+			__gitcomp_nl "$(__git_tags)"
 			return
 			;;
 		-f)
@@ -2576,13 +2600,13 @@ _git_tag ()
 		;;
 	-*|tag)
 		if [ $f = 1 ]; then
-			__gitcomp "$(__git_tags)"
+			__gitcomp_nl "$(__git_tags)"
 		else
 			COMPREPLY=()
 		fi
 		;;
 	*)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		;;
 	esac
 }
@@ -2635,7 +2659,7 @@ _git ()
 			"
 			;;
 		*)     __git_compute_porcelain_commands
-		       __gitcomp "$__git_porcelain_commands $(__git_aliases)" ;;
+		       __gitcomp_nl "$__git_porcelain_commands $(__git_aliases)" ;;
 		esac
 		return
 	fi
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 1/9] completion: document __gitcomp()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

I always forget which argument is which, and got tired of figuring it
out over and over again.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b36f9e70..c0fb6e15 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -485,8 +485,13 @@ _get_comp_words_by_ref ()
 fi
 fi
 
-# __gitcomp accepts 1, 2, 3, or 4 arguments
-# generates completion reply with compgen
+# Generates completion reply with compgen, appending a space to possible
+# completion words, if necessary.
+# It accepts 1 to 4 arguments:
+# 1: List of possible completion words.
+# 2: A prefix to be added to each possible completion word (optional).
+# 3: Generate possible completion matches for this word (optional).
+# 4: A suffix to be appended to each possible completion word (optional).
 __gitcomp ()
 {
 	local cur_="$cur"
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 0/9] ref completion optimizations, fixes, and cleanups
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor

Hi,

This series aims to improve the completion of refs & co.

This one is the most important in the series; it takes some shortcuts
to make completing large number of refs faster (it's also faster for
git.git, but it's unnoticeable).

  [2/9] completion: optimize refs completion

The following three make __git_refs() handle local and remote
repositories more consistently, and also clean up the remote-handling
code part of __git_refs().  They likely make things a bit faster, but
since the code path usually involves network communication I didn't
run any benchmarks.

  [3/9] completion: make refs completion consistent for local and remote
          repos
  [4/9] completion: improve ls-remote output filtering in __git_refs()
  [5/9] completion: support full refs from remote repositories

The following two do similar cleanups in __git_refs_remotes() than 3/9
and 4/9 in __git_refs().

  [6/9] completion: query only refs/heads/ in __git_refs_remotes()
  [7/9] completion: improve ls-remote output filtering in
          __git_refs_remotes()

A silly while-at-it optimization; the delay eliminated by this one was
annoying when testing 6/9 and 7/9.

  [8/9] completion: fast initial completion for config 'remote.*.fetch'
          value

And finally remove some bitrotted code.

  [9/9] completion: remove broken dead code from __git_heads() and
          __git_tags()


This series is meant to be applied on the merge of master and 77653abd
(completion: commit --fixup and --squash, 2011-10-06) from pu, and the
patch in

  Message-ID: <20111008010634.GB11561@goldbirke>
  (http://article.gmane.org/gmane.comp.version-control.git/183131)

from last night applied.  There will be two easily fixable conflicts
when applied directly on top of current master.


Best,
Gábor


SZEDER Gábor (9):
  completion: document __gitcomp()
  completion: optimize refs completion
  completion: make refs completion consistent for local and remote
    repos
  completion: improve ls-remote output filtering in __git_refs()
  completion: support full refs from remote repositories
  completion: query only refs/heads/ in __git_refs_remotes()
  completion: improve ls-remote output filtering in
    __git_refs_remotes()
  completion: fast initial completion for config 'remote.*.fetch' value
  completion: remove broken dead code from __git_heads() and
    __git_tags()

 contrib/completion/git-completion.bash |  200 +++++++++++++++++---------------
 1 files changed, 109 insertions(+), 91 deletions(-)

-- 
1.7.7.187.ga41de

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox