Closed mmitch closed 3 years ago
@ranma Care to have a look at this?
I have removed most of the duplications between gbsplay.c
and xgbsplay.c
by moving common code to player.c
. From my point of view the duplications where the biggest showstopper preventing a merge to master.
The X11 UI is still very basic, but it works.
With the code deduplicated, xgbsplay in master should not lag behind gbsplay like this branch did in the last 1.5 years.
Travis CI will build both gbsplay and xgbsplay, so at least compile-time problems should be caught by that.
The default build configuration is still "only gbsplay", for xgbsplay you need to ./configure --with-xgsplay
, so any problems with the xgbsplay code shouldn't bother anybody out there.
Merging #22 into master will increase coverage by
0.16%
. The diff coverage is40.71%
.
@@ Coverage Diff @@
## master #22 +/- ##
==========================================
+ Coverage 41.97% 42.13% +0.16%
==========================================
Files 18 19 +1
Lines 2716 2727 +11
==========================================
+ Hits 1140 1149 +9
- Misses 1576 1578 +2
Impacted Files | Coverage Δ | |
---|---|---|
player.c | 39.18% <39.18%> (ø) |
|
gbsplay.c | 33.33% <87.50%> (-2.50%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update a0e5459...3fcf170. Read the comment docs.
Rebased to current master, please review.
I think the missed code coverage target for the patch can be ignored – I did not even know we had coverage targets ;-)
I've created a small demo implementation of an X11 gbsplay client. Now that we have the .desktop file integration, it is quite nice to just click and play a file – and even nicer when gbsplay does not spawn an extra terminal :-)
Please share your thoughts on whether this works for you, if it is a good or bad idea and if it should be merged to the master branch (and when it is, if it should be enabled or disabled by default).
how to use
Run
configure --with-xgbsplay
, thenmake
,make install
as usual. As bothgbsplay.desktop
andxgbsplay.desktop
will be installed and there is no way of providing any precedence, you might have to manually switch*.gbs
files from gbsplay to xgbsplay - both should be listed as supported applications.limitations and TODOs
gbsplay.c
intoxgbsplay.c
and then made some minor changes - that's a lot of duplicate code that should be refactorediodumper
,midi
) as they are of little use with a graphical interface but I did not do that yet because I'd have to change or duplicateplugout_select_by_name()
andplugout_list_plugins()
(neither is prepared to accept a filter)1
…4
) are currently unsupportedAaaaand the interface is plain ugly. This has multiple reasons: