Re: [PATCH v5 03/10] add-interactive.c: implement list_modified

>> +	for (i = 0; i < stat.nr; i++) {
>> +		struct file_stat *entry;
>> +		const char *name = stat.files[i]->name;
>> +		unsigned int hash = strhash(name);
>> +
>> +		entry = hashmap_get_from_hash(&s->file_map, hash, name);
>> +		if (!entry) {
>> +			FLEX_ALLOC_STR(entry, name, name);
>> +			hashmap_entry_init(entry, hash);
>> +			hashmap_add(&s->file_map, entry);
>> +		}
>
> The path may already be in the collection_status.file_map from the
> previous run when "diff-index --cached" is run, in which case we avoid
> adding it twice, which makes sense.
>
>> +		if (s->phase == WORKTREE) {
>> +			entry->worktree.added = stat.files[i]->added;
>> +			entry->worktree.deleted = stat.files[i]->deleted;
>> +		} else if (s->phase == INDEX) {
>> +			entry->index.added = stat.files[i]->added;
>> +			entry->index.deleted = stat.files[i]->deleted;
>> +		}
>
> As the set of phases will not going to grow, not having the final
> 'else BUG("phase is neither WORKTREE nor INDEX");' here is OK.
>
> But stepping back a bit, if we know we will not grow the phases,
> then it may be simpler *and* equally descriptive to rename .phase
> field to a boolean ".collecting_from_index" that literally means
> "are we collecting from the index?" and that way we can also get rid
> of the enum.

> Grep for "unborn" in the codebase.  It probably makes sense to call
> it on_unborn_branch() instead.
>
> 	static int on_unborn_branch(void)
> 	{
> 		struct object_id oid;
> 		return !!get_oid("HEAD", &oid);
> 	}
>
> Eventually, the users of "unborn" in sequencer.c and builtin/reset.c
> may want to share the implementation but the helper is so small that
> we probably should not worry about it until the topic is done and
> leave it for a later clean-up.

>> +static struct collection_status *list_modified(struct repository *r, const char *filter)
>> +{
>> +	int i = 0;
>> +	struct collection_status *s = xcalloc(1, sizeof(*s));
>> +	struct hashmap_iter iter;
>> +	struct file_stat **files;
>> +	struct file_stat *entry;
>> +
>> +	if (repo_read_index(r) < 0) {
>> +		printf("\n");
>> +		return NULL;
>> +	}
>> +
>> +	s->reference = get_diff_reference();
>> +	hashmap_init(&s->file_map, hash_cmp, NULL, 0);
>> +
>> +	collect_changes_worktree(s);
>> +	collect_changes_index(s);
>> +
>> +	if (hashmap_get_size(&s->file_map) < 1) {
>> +		printf("\n");
>> +		return NULL;
>> +	}