Re: What's cooking in git.git (Jul 2015, #01; Wed, 1)
W dniu 2015-07-22 o 15:19, Tony Finch pisze: Jakub Narębski jna...@gmail.com wrote: A question about implementation: why emptying $path_info in evaluate_path_info()? That was for consistency with other parts of the subroutine which (mostly) remove items from the global $path_info variable when they are added to %input_params. But since $path_info isn't used after it has been parsed, I suppose it is redundant. If it is for consistency, better leave it in my opinion. - I think that people would want to be able to configure how many levels of directory hierarchy gets turned into categories. Perhaps only top level should be turned into category? Deep hierarchies means deep categories (usually with very few repositories) with current implementation. Good question. I was assuming flat-ish directory hierarchies, but that's clearly not very true, e.g. https://git.kernel.org/cgit/ I think it would be right to make this a %feature since categories already nearly fit the %feature per-project override style. On the other hand $projects_list_group_categories is a global gitweb configuration variable, and $projects_list_directory_is_category was patterned after it. Yes... Which do you prefer? :-) Hmmm... does it makes sense to have per-repository override? If yes, then we need to use %features. If not... I am not sure, %features is newer than global (or rather package) variables for gitweb configuration, which must be left for legacy config support (and few are needed before %features are parsed). A few thoughts about implementation: Helpful, thanks! - can we turn category header into link even if the category didn't came from $projects_list_directory_is_category? That would mean changing the project filter to match categories as well as paths. I don't know if this is the right thing to do; perhaps it is, because the current behaviour of my category headings is a bit surprising. At the moment, clicking on the git category heading on the page linked below takes you to a page that does not list all the repos that were under the category heading on the main page. https://git.csx.cam.ac.uk/x/ucs/ I thought gitweb had a way to list all projects belonging to given category, but I see that it doesn't. So you need to find out if 'category' came from category or from pathname, to decide whether to link it using 'projects_list' action and 'project_filter' parameter (or their PATH_INFO version), or not. This can be done either by checking that category name is directory (though we could have false positives here), or when adding categories denote where it came from (e.g. with additional field). I think the second is better, if we are to hyperlink category-from-pathname headings. There is interesting corner case: what if some projects use category, and some have the same category from pathname? Clicking on category if hyper-linked would show only a subset of projects inside category. (I think this is the oddity you noticed.) - even if $projects_list_directory_is_category is true, the category could came from 'category' file, or otherwise manually set category, though I wonder how we can easily detect this... Yes - I use this to list my personal/experimental repos alongside the production repos. I'm not sure why gitweb would need to detect this or what it would do in response. At the moment it just works, apart from the oddity with categories vs project filters i described above. What if there is synthetic category that has no representative in the path hierarchy? Then project_filter link would lead to strange empty list of projects... For example http://git.zx2c4.com/ (cgit) uses Mirrors category... We could either abuse project_filter for categories, or add a new query parameter project_category or cat in short. In either case it would not have PATH_INFO URL unless category came from directory. Food for thought -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2015, #01; Wed, 1)
Jakub Narębski jna...@gmail.com wrote: Thanks for the review! * tf/gitweb-project-listing (2015-03-19) 5 commits - gitweb: make category headings into links when they are directories - gitweb: optionally set project category from its pathname - gitweb: add a link under the search box to clear a project filter - gitweb: if the PATH_INFO is incomplete, use it as a project_filter Update gitweb to make it more pleasant to deal with a hierarchical forest of repositories. By the way, you can see this patch series in action at https://git.csx.cam.ac.uk/x/ucs/ Second one, gitweb: if the PATH_INFO is incomplete, use it as a project_filter looks interesting and quite useful. Though it doesn't do much: it allows for handcrafted URL, and provides mechanism to create breadcrumbs. It doesn't use this feature in its output... Well, I think it doesn't: I cannot check it at this moment. Hmm, I think this means I need a better commit message. This patch fixes the ugly query-parameter URLs in the breadcrumbs that you get even in path-info mode. Have a look at the breadcrumbs on the following pages: https://git.csx.cam.ac.uk/g/ucs/git/git.git (unpatched) https://git.csx.cam.ac.uk/x/ucs/git/git.git (patched) If you click on the antepenultimate /git/ in the breadcumbs you get query parameters without the patch and path_info with the patch. With the patch the breadcrumbs match the URL. What is missing is a support for query parameters path, and not only path info. Query parameter support is already present, in the form of project filters. Thought some thought is needed for generating (or not) breadcrumbs if path_info is turned off. That already works in unpatched gitweb. The third, gitweb: add a link under the search box to clear a project filter notices a problem... then solves it in strange way. IMVHO a better solution would be to add List all projects URL together with / (or other separator) conditionally, if $project_filter is set. Or have List all projects and add List projects$limit if $project_filter is set. Yes, that is exactly what the patch does. I used a suffix if to align the print statements and markup: + if $project_filter; Compare and contrast the search box on these pages: https://git.csx.cam.ac.uk/g/ucs/?a=project_list;pf=u/fanf2 https://git.csx.cam.ac.uk/x/ucs/u/fanf2/ Perhaps you would prefer the following? --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -5549,10 +5549,14 @@ sub git_project_search_form { /span\n . $cgi-submit(-name = 'btnS', -value = 'Search') . $cgi-end_form() . \n . - $cgi-a({-href = href(project = undef, searchtext = undef, -project_filter = $project_filter)}, - esc_html(List all projects$limit)) . br /\n; - print /div\n; + $cgi-a({-href = $my_uri}, esc_html(List all projects)); + if ($project_filter) { + print / . + $cgi-a({-href = href(project = undef, action = project_list, + project_filter = $project_filter)}, + esc_html(List projects$limit)); + } + print br /\n/div\n; } # entry for given @keys needs filling if at least one of keys in list The last two, which form the crux of this patch series, looks like a good idea, though not without a few caveats. I am talking here only about conceptual level, not about how it is coded (which has few issues as well): - I think that non-bare repositories repo/.git should be treated as one directory entry, i.e. gitweb should not create a separate category for repo/. This is admittedly a corner case, but useful for git-instaweb Yes, that's a bug, thanks for spotting it! - I think that people would want to be able to configure how many levels of directory hierarchy gets turned into categories. Perhaps only top level should be turned into category? Deep hierarchies means deep categories (usually with very few repositories) with current implementation. Good question. I was assuming flat-ish directory hierarchies, but that's clearly not very true, e.g. https://git.kernel.org/cgit/ I think it would be right to make this a %feature since categories already nearly fit the %feature per-project override style. I will send a new version of the series shortly. Tony. -- f.anthony.n.finch d...@dotat.at http://dotat.at/ Viking, North Utsire: Westerly 4 or 5, occasionally 6 at first, backing southerly 3 or 4. Moderate becoming slight. Occasional rain in north. Good, occasionally moderate.
Re: What's cooking in git.git (Jul 2015, #01; Wed, 1)
On 2015-07-22, Tony Finch wrote: Jakub Narębski jna...@gmail.com wrote: Thanks for the review! * tf/gitweb-project-listing (2015-03-19) 5 commits - gitweb: make category headings into links when they are directories - gitweb: optionally set project category from its pathname - gitweb: add a link under the search box to clear a project filter - gitweb: if the PATH_INFO is incomplete, use it as a project_filter Update gitweb to make it more pleasant to deal with a hierarchical forest of repositories. By the way, you can see this patch series in action at https://git.csx.cam.ac.uk/x/ucs/ Thanks. I don't have my computer set up completely yet (after reinstall). Second one, gitweb: if the PATH_INFO is incomplete, use it as a project_filter looks interesting and quite useful. Though it doesn't do much: it allows for handcrafted URL, and provides mechanism to create breadcrumbs. It doesn't use this feature in its output... Well, I think it doesn't: I cannot check it at this moment. Hmm, I think this means I need a better commit message. This patch fixes the ugly query-parameter URLs in the breadcrumbs that you get even in path-info mode. Have a look at the breadcrumbs on the following pages: https://git.csx.cam.ac.uk/g/ucs/git/git.git (unpatched) https://git.csx.cam.ac.uk/x/ucs/git/git.git (patched) If you click on the antepenultimate /git/ in the breadcumbs you get query parameters without the patch and path_info with the patch. With the patch the breadcrumbs match the URL. Ah. Yes, the patch itself looks all right, but it definitely needs a better (or at least enhanced) commit message if it is about *adding* path info counterpart to existing query parameter project_filter - - it is (also) about uniquifying URLs used in breadcrumbs when gitweb uses path info links. Current version is (if I have it correctly): gitweb: if the PATH_INFO is incomplete, use it as a project_filter Previously gitweb would ignore partial PATH_INFO. For example, it would produce a project list for the top URL https://www.example.org/projects/ and a project summary for https://www.example.org/projects/git/git.git but if you tried to list just the git-related projects with https://www.example.org/projects/git/ you would get a list of all projects, same as the top URL. As well as fixing that omission, this change also makes gitweb generate PATH_INFO-style URLs for project filter links, such as in the breadcrumbs. A question about implementation: why emptying $path_info in evaluate_path_info()? What is missing is a support for query parameters path, and not only path info. Query parameter support is already present, in the form of project filters. Thought some thought is needed for generating (or not) breadcrumbs if path_info is turned off. That already works in unpatched gitweb. Right. The third, gitweb: add a link under the search box to clear a project filter notices a problem... then solves it in strange way. IMVHO a better solution would be to add List all projects URL together with / (or other separator) conditionally, if $project_filter is set. Or have List all projects and add List projects$limit if $project_filter is set. Yes, that is exactly what the patch does. I used a suffix if to align the print statements and markup: + if $project_filter; Compare and contrast the search box on these pages: https://git.csx.cam.ac.uk/g/ucs/?a=project_list;pf=u/fanf2 https://git.csx.cam.ac.uk/x/ucs/u/fanf2/ Perhaps you would prefer the following? --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -5549,10 +5549,14 @@ sub git_project_search_form { /span\n . $cgi-submit(-name = 'btnS', -value = 'Search') . $cgi-end_form() . \n . - $cgi-a({-href = href(project = undef, searchtext = undef, -project_filter = $project_filter)}, - esc_html(List all projects$limit)) . br /\n; - print /div\n; + $cgi-a({-href = $my_uri}, esc_html(List all projects)); + if ($project_filter) { + print / . + $cgi-a({-href = href(project = undef, action = project_list, + project_filter = $project_filter)}, + esc_html(List projects$limit)); + } + print br /\n/div\n; } # entry for given @keys needs filling if at least one of keys in list Yes, it is eminently more readable. Postfix controls are discouraged, especially with multi-line constructs c.f. Perl::Critic::Policy::ControlStructures::ProhibitPostfixControls The last two, which form the crux of this patch series, looks like a good idea, though not without a few caveats. I am talking here only about conceptual level, not about how it is coded (which has few issues as well): - I
Re: What's cooking in git.git (Jul 2015, #01; Wed, 1)
Jakub Narębski jna...@gmail.com wrote: A question about implementation: why emptying $path_info in evaluate_path_info()? That was for consistency with other parts of the subroutine which (mostly) remove items from the global $path_info variable when they are added to %input_params. But since $path_info isn't used after it has been parsed, I suppose it is redundant. - I think that people would want to be able to configure how many levels of directory hierarchy gets turned into categories. Perhaps only top level should be turned into category? Deep hierarchies means deep categories (usually with very few repositories) with current implementation. Good question. I was assuming flat-ish directory hierarchies, but that's clearly not very true, e.g. https://git.kernel.org/cgit/ I think it would be right to make this a %feature since categories already nearly fit the %feature per-project override style. On the other hand $projects_list_group_categories is a global gitweb configuration variable, and $projects_list_directory_is_category was patterned after it. Yes... Which do you prefer? :-) A few thoughts about implementation: Helpful, thanks! - can we turn category header into link even if the category didn't came from $projects_list_directory_is_category? That would mean changing the project filter to match categories as well as paths. I don't know if this is the right thing to do; perhaps it is, because the current behaviour of my category headings is a bit surprising. At the moment, clicking on the git category heading on the page linked below takes you to a page that does not list all the repos that were under the category heading on the main page. https://git.csx.cam.ac.uk/x/ucs/ - even if $projects_list_directory_is_category is true, the category could came from 'category' file, or otherwise manually set category, though I wonder how we can easily detect this... Yes - I use this to list my personal/experimental repos alongside the production repos. I'm not sure why gitweb would need to detect this or what it would do in response. At the moment it just works, apart from the oddity with categories vs project filters i described above. Tony. -- f.anthony.n.finch d...@dotat.at http://dotat.at/ Viking, North Utsire: Westerly 4 or 5, occasionally 6 at first, backing southerly 3 or 4. Moderate becoming slight. Occasional rain in north. Good, occasionally moderate.
Re: What's cooking in git.git (Jul 2015, #01; Wed, 1)
Jakub Narębski jna...@gmail.com wrote: Food for thought Yes, very helpful, thanks. I got mobbed by other things today so I won't be able to get back to this until next week. Tony (off for a few days holiday). -- f.anthony.n.finch d...@dotat.at http://dotat.at/ Lundy, Fastnet, Irish Sea, Shannon: West or southwest, veering north later in Shannon and Fastnet, 4 or 5. Slight or moderate, occasionally rough at first in north Shannon. Rain or showers. Good, occasionally poor.
Re: What's cooking in git.git (Jul 2015, #01; Wed, 1)
On 2015-07-02 at 00:37, Junio C Hamano wrote: What's cooking in git.git (Jul 2015, #01; Wed, 1) -- * tf/gitweb-project-listing (2015-03-19) 5 commits - gitweb: make category headings into links when they are directories - gitweb: optionally set project category from its pathname - gitweb: add a link under the search box to clear a project filter - gitweb: if the PATH_INFO is incomplete, use it as a project_filter - gitweb: fix typo in man page Update gitweb to make it more pleasant to deal with a hierarchical forest of repositories. Any comments from those who use or have their own code in Gitweb? The first one is a simple typo fix (plural - singular), so it can be accepted without problems. Second one, gitweb: if the PATH_INFO is incomplete, use it as a project_filter looks interesting and quite useful. Though it doesn't do much: it allows for handcrafted URL, and provides mechanism to create breadcrumbs. It doesn't use this feature in its output... Well, I think it doesn't: I cannot check it at this moment. What is missing is a support for query parameters path, and not only path info. Though that *might* be postponed for later patch; the path info API is obvious, query params API for this feature isn't. Thought some thought is needed for generating (or not) breadcrumbs if path_info is turned off. The third, gitweb: add a link under the search box to clear a project filter notices a problem... then solves it in strange way. IMVHO a better solution would be to add List all projects URL together with / (or other separator) conditionally, if $project_filter is set. Or have List all projects and add List projects$limit if $project_filter is set. The last two, which form the crux of this patch series, looks like a good idea, though not without a few caveats. I am talking here only about conceptual level, not about how it is coded (which has few issues as well): - I think that non-bare repositories repo/.git should be treated as one directory entry, i.e. gitweb should not create a separate category for repo/. This is admittedly a corner case, but useful for git-instaweb - I think that people would want to be able to configure how many levels of directory hierarchy gets turned into categories. Perhaps only top level should be turned into category? Deep hierarchies means deep categories (usually with very few repositories) with current implementation. - New global configuration variable, or new %features entry? * tr/remerge-diff (2014-11-10) 9 commits - t4213: avoid | in sed regexp - log --remerge-diff: show what the conflict resolution changed - name-hash: allow dir hashing even when !ignore_case - merge-recursive: allow storing conflict hunks in index - merge_diff_mode: fold all merge diff variants into an enum - combine-diff: do not pass revs-dense_combined_merges redundantly - merge-recursive: -Xindex-only to leave worktree unchanged - merge-recursive: internal flag to avoid touching the worktree - merge-recursive: remove dead conditional in update_stages() log -p output learns a new way to let users inspect a merge commit by showing the differences between the automerged result with conflicts the person who recorded the merge would have seen and the final conflict resolution that was recorded in the merge. Waiting for a reroll. ($gmane/256591). Is it something that Atlassian uses as a differentiatior (instead of sending patch upstream): https://developer.atlassian.com/blog/2015/01/a-better-pull-request/ -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PUB]What's cooking in git.git (Jul 2015, #01; Wed, 1)
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: * ad/bisect-terms (2015-06-29) 10 commits ... The bottom part has been quite well cooked. Perhaps split it into two topisc and merge the earlier ones to 'next' before the rest settles. Michael's idea to make 'good/bad' more intelligent does have certain attractiveness ($gname/272867). I think it makes sense to merge the first patches soon: - bisect: don't mix option parsing and non-trivial code - bisect: simplify the addition of new bisect terms - bisect: replace hardcoded bad|good by variables - Documentation/bisect: revise overall content - Documentation/bisect: move getting help section to the end - bisect: correction of typo I have nothing to add on the last ones, but they can cook in pu a bit longer. Do you expect anything from my side? Not at this moment. Thanks for helping this topic move forward. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PUB]What's cooking in git.git (Jul 2015, #01; Wed, 1)
Junio C Hamano gits...@pobox.com writes: * ad/bisect-terms (2015-06-29) 10 commits - bisect: allow setting any user-specified in 'git bisect start' - bisect: add 'git bisect terms' to view the current terms - bisect: add the terms old/new - bisect: sanity check on terms - bisect: don't mix option parsing and non-trivial code - bisect: simplify the addition of new bisect terms - bisect: replace hardcoded bad|good by variables - Documentation/bisect: revise overall content - Documentation/bisect: move getting help section to the end - bisect: correction of typo The use of 'good/bad' in git bisect made it confusing to use when hunting for a state change that is not a regression (e.g. bugfix). The command learned 'old/new' and then allows the end user to say e.g. bisect start --term-old=fast --term=new=slow to find a performance regression. The bottom part has been quite well cooked. Perhaps split it into two topisc and merge the earlier ones to 'next' before the rest settles. Michael's idea to make 'good/bad' more intelligent does have certain attractiveness ($gname/272867). I think it makes sense to merge the first patches soon: - bisect: don't mix option parsing and non-trivial code - bisect: simplify the addition of new bisect terms - bisect: replace hardcoded bad|good by variables - Documentation/bisect: revise overall content - Documentation/bisect: move getting help section to the end - bisect: correction of typo I have nothing to add on the last ones, but they can cook in pu a bit longer. Do you expect anything from my side? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Jul 2015, #01; Wed, 1)
What's cooking in git.git (Jul 2015, #01; Wed, 1) -- Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * da/mergetool-winmerge (2015-06-19) 1 commit (merged to 'next' on 2015-06-24 at 2fb10c4) + mergetool-lib: fix default tool selection Hotfix for an earlier change already in 'master' that broke the default tool selection for mergetool. * jc/prompt-document-ps1-state-separator (2015-06-10) 1 commit (merged to 'next' on 2015-06-24 at e4d1bad) + git-prompt.sh: document GIT_PS1_STATESEPARATOR Docfix. * me/fetch-into-shallow-safety (2015-06-17) 1 commit (merged to 'next' on 2015-06-24 at 8ecc19a) + fetch-pack: check for shallow if depth given git fetch --depth=depth and git clone --depth=depth issued a shallow transfer request even to an upload-pack that does not support the capability. * mm/describe-doc (2015-06-16) 1 commit (merged to 'next' on 2015-06-24 at 75e34cc) + Documentation/describe: improve one-line summary Docfix. -- [New Topics] * jk/rev-list-no-bitmap-while-pruning (2015-07-01) 1 commit - rev-list: disable --use-bitmap-index when pruning commits A minor bugfix when pack bitmap was brought in. Will merge to 'next'. * kb/config-unmap-before-renaming (2015-06-30) 1 commit - config.c: fix writing config files on Windows network shares Will merge to 'next'. * ls/hint-rev-list-count (2015-07-01) 1 commit - rev-list: add --count to usage guide Will merge to 'next'. * mh/fast-import-get-mark (2015-07-01) 1 commit - fast-import: add a get-mark command Will merge to 'next'. * nd/dwim-wildcards-as-pathspecs (2015-07-01) 1 commit - Add tests for wildcard path vs ref disambiguation Will merge to 'next' and then to 'master'. -- [Stalled] * sg/config-name-only (2015-05-28) 3 commits - completion: use new 'git config' options to reliably list variable names - SQUASH - config: add options to list only variable names git config --list output was hard to parse when values consist of multiple lines. Introduce a way to show only the keys. Adding a single --name-only option may be a better way to go than adding two new options. Expecting a reroll. * kk/log-merges-config (2015-04-21) 5 commits - bash-completion: add support for git-log --merges= and log.merges - t4202-log: add tests for --merges= - Documentation: add git-log --merges= option and log.merges config. var - log: honor log.merges= option - revision: add --merges={show|only|hide} option git log (but not other commands in the log family) learned to pay attention to the log.merges configuration variable that can be set to show (the normal behaviour), only (hide non-merge commits), or hide (hide merge commits). --merges=(show|only|hide) can be used to override the setting from the command line. The documentation may need to be updated once more ($gmane/267250). Waiting for a reroll. * mg/httpd-tests-update-for-apache-2.4 (2015-04-08) 2 commits - t/lib-git-svn: check same httpd module dirs as lib-httpd - t/lib-httpd: load mod_unixd This is the first two commits in a three-patch series $gmane/266962 Will be rerolled. with updated log message ($gmane/268061). * mh/numparse (2015-03-19) 14 commits - diff_opt_parse(): use convert_i() when handling --abbrev=num - diff_opt_parse(): use convert_i() when handling -lnum - opt_arg(): simplify pointer handling - opt_arg(): report errors parsing option values - opt_arg(): use convert_i() in implementation - opt_arg(): val is always non-NULL - builtin_diff(): detect errors when parsing --unified argument - handle_revision_opt(): use convert_ui() when handling --abbrev= - strtoul_ui(), strtol_i(): remove functions - handle_revision_opt(): use convert_i() when handling -digit - handle_revision_opt(): use skip_prefix() in many places - write_subdirectory(): use convert_ui() for parsing mode - cacheinfo_callback(): use convert_ui() when handling --cacheinfo - numparse: new module for parsing integral numbers Many codepaths use unchecked use of strtol() and friends (or even worse, atoi()). Introduce a set of wrappers that try to be more careful. Expecting a reroll. ($gmane/268058). * tf/gitweb-project-listing (2015-03-19) 5 commits - gitweb: make category headings into links when they are directories - gitweb: optionally set project category from its pathname - gitweb: add a link under the search box to clear a project filter - gitweb: if the PATH_INFO is incomplete, use it as a project_filter - gitweb: fix typo in man page