macgitver / MacGitverModules

DEPRECATED: Modules for MacGitver
5 stars 1 forks source link

Sc/refsview #99

Closed scunz closed 9 years ago

scunz commented 9 years ago

An alternative approach to improve the RefsView.

This is based upon ngf/model-update and includes the compile-fix commit, which should be removed from the branches by rebasing them to development.

This approach doesn't consider stashes and namespaces yet. The support for stashes is lacking in RepoMan and probably in GitWrap. For namespaces I'm simply not sure how to display them correctly. The basic problem is, that each namespace could have both branches and tags (as well as other namespaces, in fact).

But we're actually having the same problem with remotes, where we should show a separate tree for tags.

antis81 commented 9 years ago

Here's what it looks like in Ubuntu :+1: mgv-ref-tree-sc

I just compiled and didn't look deeper into this code yet. Btw. Compared to the other branch, drawing with current code is actually performing faster, which you can easily see by dragging the splitters.

For the visual appearance, I have some questions / notes:

scunz commented 9 years ago

Btw. Compared to the other branch, drawing with current code is actually performing faster, which you can easily see by dragging the splitters.

Which code are you talking about? this code omits your reading of the repository from disc on each drawing, so it is faster.

I think, the icons are "cluttering" the view. Can you try putting them only to the headers? (Icons for RefTreeNodes should be kept, I think.)

No, these icons don't make any sense in the header. And having an icon for all items finally removes that annoying cluttering that made it impossible to spot the actual hierarchy. Everything is now aligned as it usually is in a tree.

I really like to have the colors back. But, it's not finished, is it?

No, this isn't done. The underlying data model doesn't carry that information and I don't want to reintroduce the code that reads this information from disc on every drawing operation.

Same for the bold font, that marks the active branch.

Same cause, same solution. The model has to carry this data, determined once during a refresh and updated with events accordingly.

antis81 commented 9 years ago

Which code are you talking about? this code omits your reading of the repository from disc on each drawing, so it is faster.

Yes, this is exactly what I was thinking about :). There's some other aspects to: i.e. you kicked the sort model layer. But this is the main point. It's not too bad, as the lookup is always done on the items drawn on screen (which is always <50). Ah ... I'm not writing any more, until I reviewed the code :).

... And having an icon for all items finally removes that annoying cluttering that made it impossible to spot the actual hierarchy. Everything is now aligned as it usually is in a tree.

I'll adapt to this and see what future brings. :smile:

The underlying data model doesn't carry that information and I don't want to reintroduce the code that reads this information from disc on every drawing operation.

Happy to hear that. We need to discuss the RM::Repo::heads() stuff, I introduced in libMacGitverCore -> PR 22/23 and make it right :). I.e.: Keep the HEAD-ref as a separate instance - out of the collections. Lookup and compare would be fast, furious and even cool :).

In current implementation that means:

QVariant RefBranch::data(int role) const
{
    Q_ASSERT(mObject);

    switch (role) {
    case Qt::FontRole:
        if( isCurrentBranch() )
        {
            QFont f;
            f.setBold( true );
            return f;
        }
        #endif
        break;

    case RefItem::RowBgGradientRole:
        if ( equalsHeadId() )
        {
            QColor back = isCurrentBranch()
                          ? QColor::fromHsl(35, 255, 190)
                          : QColor::fromHsl(35, 255, 190).lighter(130);
            return back;
        }
        #endif
        break;

    case Qt::EditRole:
        return object()->name();
    }

    return RefItemObject::data(role);
}

// spice it up with some private salt & pepper :)

const RM::Ref* RefBranch::findHEAD( const RM::Repo* repo ) {
    // this can be optimized in libMacGitverCore, by providing a RM::Repo::head() method
    // Anyways, this code will work!

    RM::Ref* HEAD = NULL;
    const QString headStr = QStringLiteral("HEAD");
    foreach ( RM::Ref* candidate, repo->heads().childObjects<RM::Ref>() ) {
        if ( candidate.fullName() == headStr ) {
            HEAD = candidate;
            break;
        }
    }

    return HEAD;
}

bool RefBranch::isCurrentBranch() const
{
    const RM::Repo* repo = mObject->repository();
    const RM::Ref* HEAD = findHEAD( repo );
    // Q_ASSERT( HEAD );

    return HEAD && HEAD->symbolicTarget() == object()->fullName();
}

bool RefBranch::equalsHeadId() const
{
    const RM::Repo* repo = mObject->repository();
    const RM::Ref* HEAD = findHEAD( repo );
    // Q_ASSERT( HEAD );

    return HEAD && HEAD->id() == object()->id();
}
antis81 commented 9 years ago

This needs rebasing due to the macro changes in GitWrap. Otherwise, compiler will complain about a missing semicolon.

scunz commented 9 years ago

This needs rebasing due to the macro changes in GitWrap. Otherwise, compiler will complain about a missing semicolon.

Yepp. Though my local branch still has that commit around - I never start to develop on the "development" branch; not even for fixes :)

scunz commented 9 years ago

Rebase done :)

antis81 commented 9 years ago

... I never start to develop on the "development" branch; not even for fixes :)

Maybe that's why lg2 renamed their "development" branch back to "master" :smile:.

Rebase done :)

It's funny to see the website updating while writing in this box :smile:. Thanks!