[fileviewgitplugin] Add log action
ClosedPublic

Authored by anthonyfieroni on Sat, Feb 3, 9:51 AM.

Details

Summary

QWebEngine
pro:

  1. Sticky header
  2. Fancy bottom border
  3. Ability for better redesign

cons:

  1. Hugh dependency

QTextBrowser
pro:

  1. Lightweight

cons:

  1. Basic design
  2. Even align not work in table despite docs
Test Plan

WebEngine

TextBrowser

Diff Detail

Repository
R449 Plugins for Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
anthonyfieroni requested review of this revision.Sat, Feb 3, 9:51 AM
anthonyfieroni created this revision.

Interesting. Not too fond of using Qt WebEngine, though. Can this perhaps be turned into a QTableView instead?

The main idea behind WebEngineView is that git to generate content, sticky header and only bottom border. Furthermore the code still looks simple as is.

I have also idea to draw graph at left side but i don't find any "easy way" to do it :D

Small css fixes.

elvisangelaccio requested changes to this revision.Sat, Feb 3, 3:57 PM

I like the idea, but please fix the dependency handling

CMakeLists.txt
15 ↗(On Diff #26439)

Requiring WebEngine just for a git log plugin is too much. It should be an optional build-time dependency.

This revision now requires changes to proceed.Sat, Feb 3, 3:57 PM

I concur that WebEngine is a bit overkill for a table...

I can give try to QTextBrowser?

anthonyfieroni edited the summary of this revision. (Show Details)
anthonyfieroni edited the test plan for this revision. (Show Details)

Use QTextBrowser instead of QWebEngine

ngraham added a subscriber: ngraham.Sat, Feb 3, 7:15 PM

+1 for QTextBrowser

Ping, is it good to go in?

Can you please rebase the patch? It doesn't apply currently

git/fileviewgitplugin.cpp
421

QLatin1Starting here (because startsWith() has an overload for it).

433–435

Why not using the new C++11 syntax:

QStringList {QStringLiteral("foo"), QStringLiteral("bar"), ...}
458

Hmm this should probably not be translated. If you are using git, you know what "git log" means ;)

Can you please rebase the patch? It doesn't apply currently

Ignore this, it applies fine. :)

Is it normal that the dolphin process does not quit if I close the dolphin window but not the Git Log window?

git/fileviewgitplugin.cpp
421

Actually, maybe we can just check if the scheme() of the url is "rev".

anthonyfieroni added a comment.EditedThu, Feb 8, 9:03 PM

Is it normal that the dolphin process does not quit if I close the dolphin window but not the Git Log window?

Yes, other plugin windows have a same behavior, they are parent-less since plugin isn't a widget.

git/fileviewgitplugin.cpp
433–435

It does not matter in that case, about me.

458

I'll update to

xi18nd("@action:intitle", "<application>Git</application> Log")

but should check if intitle presents :)

anthonyfieroni marked 6 inline comments as done.
elvisangelaccio accepted this revision.Fri, Feb 9, 8:23 PM
elvisangelaccio added inline comments.
git/fileviewgitplugin.cpp
420

QLatin1String here (faster with != operator)

This revision is now accepted and ready to land.Fri, Feb 9, 8:23 PM
This revision was automatically updated to reflect the committed changes.