D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D20166#448914 , @cullmann wrote:
  
  > The CI is a bit unhappy with the gzip depedency on Windows.
  >  Might one just write a minimal KArchive based gzip'er for this? gzip isn't 
there on any normal Windows machine, even if you have libz.
  
  
  D20499  is currently in the works, 
needing people != me who use windows to test, please :)

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20166

To: kossebau, #plasma, #vdg, ngraham
Cc: cullmann, bruns, GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, 
michaelh

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Christoph Cullmann]

cullmann added a comment.


  The CI is a bit unhappy with the gzip depedency on Windows.
  Might one just write a minimal KArchive based gzip'er for this? gzip isn't 
there on any normal Windows machine, even if you have libz.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20166

To: kossebau, #plasma, #vdg, ngraham
Cc: cullmann, bruns, GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, 
michaelh

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Friedrich W. H. Kossebau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:38ba8beee3a5: Keep desktoptheme SVG files uncompressed in 
repo, install svgz (authored by kossebau).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D20166?vs=55383=56025#toc

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20166?vs=55383=56025

REVISION DETAIL
  https://phabricator.kde.org/D20166

AFFECTED FILES
  CMakeLists.txt
  cmake/Findgzip.cmake
  src/desktoptheme/CMakeLists.txt
  src/desktoptheme/air/CMakeLists.txt
  src/desktoptheme/air/dialogs/background.svg
  src/desktoptheme/air/dialogs/background.svgz
  src/desktoptheme/air/dialogs/kickoff.svg
  src/desktoptheme/air/dialogs/kickoff.svgz
  src/desktoptheme/air/dialogs/krunner.svg
  src/desktoptheme/air/dialogs/krunner.svgz
  src/desktoptheme/air/icons/akonadi.svg
  src/desktoptheme/air/icons/akonadi.svgz
  src/desktoptheme/air/icons/akregator.svg
  src/desktoptheme/air/icons/akregator.svgz
  src/desktoptheme/air/icons/amarok.svg
  src/desktoptheme/air/icons/amarok.svgz
  src/desktoptheme/air/icons/applications.svg
  src/desktoptheme/air/icons/applications.svgz
  src/desktoptheme/air/icons/apport.svg
  src/desktoptheme/air/icons/apport.svgz
  src/desktoptheme/air/icons/audio.svg
  src/desktoptheme/air/icons/audio.svgz
  src/desktoptheme/air/icons/battery.svg
  src/desktoptheme/air/icons/battery.svgz
  src/desktoptheme/air/icons/bookmarks.svg
  src/desktoptheme/air/icons/bookmarks.svgz
  src/desktoptheme/air/icons/computer.svg
  src/desktoptheme/air/icons/computer.svgz
  src/desktoptheme/air/icons/configure.svg
  src/desktoptheme/air/icons/configure.svgz
  src/desktoptheme/air/icons/device.svg
  src/desktoptheme/air/icons/device.svgz
  src/desktoptheme/air/icons/edit.svg
  src/desktoptheme/air/icons/edit.svgz
  src/desktoptheme/air/icons/kdeconnect.svg
  src/desktoptheme/air/icons/kdeconnect.svgz
  src/desktoptheme/air/icons/keyboard.svg
  src/desktoptheme/air/icons/keyboard.svgz
  src/desktoptheme/air/icons/kget.svg
  src/desktoptheme/air/icons/kget.svgz
  src/desktoptheme/air/icons/klipper.svg
  src/desktoptheme/air/icons/klipper.svgz
  src/desktoptheme/air/icons/konv_message.svg
  src/desktoptheme/air/icons/konv_message.svgz
  src/desktoptheme/air/icons/konversation.svg
  src/desktoptheme/air/icons/konversation.svgz
  src/desktoptheme/air/icons/kopete.svg
  src/desktoptheme/air/icons/kopete.svgz
  src/desktoptheme/air/icons/korgac.svg
  src/desktoptheme/air/icons/korgac.svgz
  src/desktoptheme/air/icons/kpackagekit.svg
  src/desktoptheme/air/icons/kpackagekit.svgz
  src/desktoptheme/air/icons/ktorrent.svg
  src/desktoptheme/air/icons/ktorrent.svgz
  src/desktoptheme/air/icons/nepomuk.svg
  src/desktoptheme/air/icons/nepomuk.svgz
  src/desktoptheme/air/icons/network.svg
  src/desktoptheme/air/icons/network.svgz
  src/desktoptheme/air/icons/notification.svg
  src/desktoptheme/air/icons/notification.svgz
  src/desktoptheme/air/icons/preferences.svg
  src/desktoptheme/air/icons/preferences.svgz
  src/desktoptheme/air/icons/printer.svg
  src/desktoptheme/air/icons/printer.svgz
  src/desktoptheme/air/icons/quassel.svg
  src/desktoptheme/air/icons/quassel.svgz
  src/desktoptheme/air/icons/slc.svg
  src/desktoptheme/air/icons/slc.svgz
  src/desktoptheme/air/icons/start.svg
  src/desktoptheme/air/icons/start.svgz
  src/desktoptheme/air/icons/system.svg
  src/desktoptheme/air/icons/system.svgz
  src/desktoptheme/air/icons/view.svg
  src/desktoptheme/air/icons/view.svgz
  src/desktoptheme/air/icons/wallet.svg
  src/desktoptheme/air/icons/wallet.svgz
  src/desktoptheme/air/opaque/dialogs/background.svg
  src/desktoptheme/air/opaque/dialogs/background.svgz
  src/desktoptheme/air/opaque/dialogs/krunner.svg
  src/desktoptheme/air/opaque/dialogs/krunner.svgz
  src/desktoptheme/air/opaque/widgets/extender-background.svg
  src/desktoptheme/air/opaque/widgets/extender-background.svgz
  src/desktoptheme/air/opaque/widgets/panel-background.svg
  src/desktoptheme/air/opaque/widgets/panel-background.svgz
  src/desktoptheme/air/opaque/widgets/tooltip.svg
  src/desktoptheme/air/opaque/widgets/tooltip.svgz
  src/desktoptheme/air/translucent/dialogs/background.svg
  src/desktoptheme/air/translucent/dialogs/background.svgz
  src/desktoptheme/air/translucent/dialogs/krunner.svg
  src/desktoptheme/air/translucent/dialogs/krunner.svgz
  src/desktoptheme/air/translucent/widgets/extender-background.svg
  src/desktoptheme/air/translucent/widgets/extender-background.svgz
  src/desktoptheme/air/translucent/widgets/panel-background.svg
  src/desktoptheme/air/translucent/widgets/panel-background.svgz
  src/desktoptheme/air/translucent/widgets/tooltip.svg
  src/desktoptheme/air/translucent/widgets/tooltip.svgz
  src/desktoptheme/air/widgets/action-overlays.svg
  src/desktoptheme/air/widgets/action-overlays.svgz
  src/desktoptheme/air/widgets/actionbutton.svg
  src/desktoptheme/air/widgets/actionbutton.svgz
  

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Sorry, I should learn to read...
  
  When following the instructions, this works flawlessly. Indeed, let's do it 
for 5.58 so we have lots of time for testing. Thanks for this very nice patch, 
@kossebau!

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  uncompressedsvginrepo

REVISION DETAIL
  https://phabricator.kde.org/D20166

To: kossebau, #plasma, #vdg, ngraham
Cc: bruns, GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, michaelh

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D20166#443916 , @ngraham wrote:
  
  > In D20166#443781 , @kossebau 
wrote:
  >
  > > If so, I would propose to merge it after KDE Frameworks 5.57 has been 
branched this WE, i.e. merge Friday next week for another 7 days of 
consideration, but with some weeks before the next release so people using 
master can test-drive things some more before release, not only me :)
  >
  >
  > Makes sense to me. That said, I just gave this a whirl and got a CMake 
error immediately:
  >
  >   CMake Error at src/desktoptheme/CMakeLists.txt:30 (message):
  > No files passed when calling plasma_install_desktoptheme_svgs.
  >   Call Stack (most recent call first):
  > src/desktoptheme/oxygen/CMakeLists.txt:5 
(plasma_install_desktoptheme_svgs)
  >
  
  
  Yes, because an important hint was already moved out of sight, so let my 
recite my first comment:
  
  The SVGZ -> SVG conversion of all files not included in this uploaded patch, 
to keep it reviewable.
  
  Locally do this:
  
  Store a file `svgztosvg.sh` with this content in `src/desktoptheme`:
  
#!/bin/sh

SVGZ=$1

if [ ! -f "$SVGZ" ]; then
echo "File not found: \"$SVGZ\""
exit 1
fi

SVG=${SVGZ%.svgz}.svg

zcat $SVGZ > $SVG
rm $SVGZ
  
  Then on the commandline do this:
  
cd plasma-framework/src/desktoptheme
find . -name "*.svgz" -exec sh svgztosvg.sh {} \;
  
  Once done, do e.g. this to restore SVGZ files:
  
find . -name "*.svg" -exec rm {} \;
git checkout air breeze oxygen

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20166

To: kossebau, #plasma, #vdg
Cc: bruns, GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, michaelh

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Nathaniel Graham]

ngraham added a comment.


  In D20166#443781 , @kossebau wrote:
  
  > If so, I would propose to merge it after KDE Frameworks 5.57 has been 
branched this WE, i.e. merge Friday next week for another 7 days of 
consideration, but with some weeks before the next release so people using 
master can test-drive things some more before release, not only me :)
  
  
  Makes sense to me. That said, I just gave this a whirl and got a CMake error 
immediately:
  
CMake Error at src/desktoptheme/CMakeLists.txt:30 (message):
  No files passed when calling plasma_install_desktoptheme_svgs.
Call Stack (most recent call first):
  src/desktoptheme/oxygen/CMakeLists.txt:5 
(plasma_install_desktoptheme_svgs)

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20166

To: kossebau, #plasma, #vdg
Cc: bruns, GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, michaelh

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Recent example where plain SVGs in the repo would have made the change 
immdiate visible, as only the strings of some id attributes where changed (to 
match the new icon names), but instead all we see with our tool is an exchange 
of the complete binary blob:  D20261 
  
  So, do I correctly see that people are rather positive about this change, at 
least no-one objecting it? If so, I would propose to merge it after KDE 
Frameworks 5.57 has been branched this WE, i.e. merge Friday next week for 
another 7 days of consideration, but with some weeks before the next release so 
people using master can test-drive things some more before release, not only me 
:)
  
  Who feels like giving  explicit +1 or the needed Accepts? :)

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20166

To: kossebau, #plasma, #vdg
Cc: bruns, GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, michaelh

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Friedrich W. H. Kossebau]

kossebau updated this revision to Diff 55383.
kossebau added a comment.


  Remove last bit about stripping that was still in the patch

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20166?vs=55231=55383

BRANCH
  uncompressedsvginrepo

REVISION DETAIL
  https://phabricator.kde.org/D20166

AFFECTED FILES
  CMakeLists.txt
  cmake/Findgzip.cmake
  src/desktoptheme/CMakeLists.txt
  src/desktoptheme/air/CMakeLists.txt
  src/desktoptheme/breeze/CMakeLists.txt
  src/desktoptheme/oxygen/CMakeLists.txt

To: kossebau, #plasma, #vdg
Cc: bruns, GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, michaelh

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Noah Davis]

ndavis added a comment.


  In D20166#441863 , @kossebau wrote:
  
  > IMHO the SVG files in the repo may contain comments and metadatas, for 
further work on them. Like source code also has all the oomments and metadata.
  >  Only the deployed graphics resource should then be limited and optimized 
for what is needed at runtime.
  >
  > At least I had dealt with quite some "final rendered" SVG files I would 
have liked to modify for some needs, but could hardly as the base resources 
where not available (and if it was only guides & Co.).
  
  
  I can see where you're coming from, but guides left by other people aren't 
that useful in my experience. I still have to guess how they made their shapes 
in the first place and there
  's no comment field that's visible from within Inkscape. Nobody ever puts 
comments in the code either. As long as you know how to use a Grid, 
Group/Ungroup and Combine/Break Apart, it's not hard to edit most existing 
icons. The few that are hard would not be improved by in-code comments or 
guides.
  
  In D20166#441875 , @bruns wrote:
  
  > [...] Also, stripping any whitespace makes diffs of minor changes quite 
large, as diffs align on lines, not tokens.
  
  
  Then we can use optimization rules that auto format and indent everything. It 
will expand the size of color icons a lot more than monochrome icons, but maybe 
we can live with that.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20166

To: kossebau, #plasma, #vdg
Cc: bruns, GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, michaelh

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Stefan Brüns]

bruns added a comment.


  In D20166#441854 , @ndavis wrote:
  
  > In D20166#441807 , @ngraham 
wrote:
  >
  > > Strictly speaking, there's no need to optimize the SVGs as a part of 
this, so maybe, yeah, let's split that out into a separate feature (and one 
that the Breeze Icons repo would ultimately be interested in as well).
  >
  >
  > Yes. Ideally, designers should optimize their work before it is landed or 
at least not submit an SVG with tons of metadata.
  
  
  On the other hand, this makes it harder for everyone else to do any changes 
later - think of e.g guides used for alignement. Also, stripping any whitespace 
makes diffs of minor changes quite large, as diffs align on lines, not tokens.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20166

To: kossebau, #plasma, #vdg
Cc: bruns, GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, michaelh

D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz

[Friedrich W. H. Kossebau]

kossebau retitled this revision from "Keep desktoptheme SVG files uncompressed 
in repo, install (stripped) svgz" to "Keep desktoptheme SVG files uncompressed 
in repo, install svgz".
kossebau edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D20166

To: kossebau, #plasma, #vdg
Cc: GB_2, ndavis, ngraham, fvogt, kde-frameworks-devel, michaelh, bruns