D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  If you want to use a library, then it would be better to mark it as a 
dependency in CMake rather than bundling it here.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe abandoned this revision.
guoyunhe added a comment.


  Using a library can save a lot of time. Just like many simple web pages use 
jQuery, D3 , etc.
  
  I don't have the ability to write it all by hands from scratch...

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  At the minimum, I'd like to see the original/un-minified version rather than 
the illegible version here that's vulnerable to attack. But why do we even need 
it in the first place? The original is 12,000 lines of Javascript. Is it 
actually necessary to have such a thing just for building a simple webpage of 
icons? Seems like overkill to me.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#456869 , @ltoscano wrote:
  
  > The documentation website of krita is a different type of repository. This 
project is going to be distributed and compiled by the distributions, and many 
of them will have to patch out the minified javascript and replace it with a 
dependency on a proper packaged version. The idea is that the minified 
(obscured) version is not the preferred form of distribution (not the "source 
code"), more or less. So special care should be taken here.
  
  
  The web page is only used by developer/designer. End users don't need it. It 
won't be executed during packaging or installation. It won't be installed, 
either. Here isn't cmake rule to execute or install it.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Luigi Toscano]

ltoscano added a comment.


  The documentation website of krita is a different type of repository. This 
project is going to be distributed and compiled by the distributions, and many 
of them will have to patch out the minified javascript and replace it with a 
dependency on a proper packaged version. The idea is that the minified 
(obscured) version is not the preferred form of distribution (not the "source 
code"), more or less. So special care should be taken here.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  The code is stable version Vue.js. Source is here 
https://github.com/vuejs/vue/blob/v2.6.10/dist/vue.min.js
  Uncompiled version is here 
https://github.com/vuejs/vue/blob/v2.6.10/dist/vue.js
  
  I used to include it from remote URL. But someone think it is not trusted. So 
I changed it to local file.
  
  Here are some KDE projects also including compiled JavaScript files 
https://cgit.kde.org/websites/docs-krita-org.git/tree/theme/static/js/modernizr.min.js

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Filip Fila]

filipf added a comment.


  In D19812#456848 , @ngraham wrote:
  
  > What is this giant javascript file with no whitespace that you're adding? I 
really don't like how it's formatted in such a way that makes the code 
practically impossible to read. This would be the perfect place to sneak in 
malware (not saying you or the author have done this, but that this kind of 
obfuscated code is a perfect vector for it).
  
  
  I have to strongly second this sentiment. What's the source of the 
`vue.min.js` file? At the very least code shouldn't be formatted this way, and 
if this was the base file I'm seeing it's not in its original form: 
https://github.com/vuejs/vue/blob/dev/dist/vue.js

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#456848 , @ngraham wrote:
  
  > What is this giant javascript file with no whitespace that you're adding? I 
really don't like how it's formatted in such a way that makes the code 
practically impossible to read. This would be the perfect place to sneak in 
malware (not saying you or the author have done this, but that this kind of 
obfuscated code is a perfect vector for it).
  
  
  It is Vue, a MVC library to make interactive single page application. I can 
provide some examples:
  
  1. WordPress https://github.com/WordPress/WordPress/tree/master/wp-includes/js
  2. Bootstrap https://github.com/twbs/bootstrap/tree/master/dist/js
  3. Phabricator (this website) 
https://github.com/phacility/phabricator/blob/master/webroot/rsrc/externals/d3/d3.min.js
  
  They all have some minified JavaScript in Git repository.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: trickyricky26, GB_2, pino, bcooksley, ngraham, kde-frameworks-devel, 
michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  What is this giant javascript file with no whitespace that you're adding? I 
really don't like how it's formatted in such a way that makes the code 
practically impossible to read. This would be the perfect place to sneak in 
malware (not saying you or the author have done this, but that this kind of 
obfuscated code is a perfect vector for it).

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: trickyricky26, GB_2, pino, bcooksley, ngraham, kde-frameworks-devel, 
michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Noah Davis]

ndavis added a comment.


  In D19812#455478 , @ngraham wrote:
  
  > TBH I wonder how useful this actually is. Hopefully some of the other folks 
involved in making icons (@ndavis, @GB_2, @trickyricky26) can comment on 
whether or not this would be useful for them.
  
  
  If I can link directly to a specific icon on that page, it can be useful for 
showing icons to others rather than only calling the icon by its name (e.g., 
`list-add`).

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: trickyricky26, GB_2, pino, bcooksley, ngraham, kde-frameworks-devel, 
michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Björn Feber]

GB_2 added a comment.


  I personally think this could be quite useful.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: trickyricky26, GB_2, pino, bcooksley, ngraham, kde-frameworks-devel, 
michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added subscribers: GB_2, trickyricky26.
ngraham added a comment.


  TBH I wonder how useful this actually is. Hopefully some of the other folks 
involved in making icons (@ndavis, @GB_2, @trickyricky26) can comment on 
whether or not this would be useful for them.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: trickyricky26, GB_2, pino, bcooksley, ngraham, kde-frameworks-devel, 
michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  Hi @ngraham @pino @bcooksley , I have updated the script/configuration. Can 
you give some further opinion?

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  Hi all,
  
  Can you check if here is anything else that needs to be changed? Thanks.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe marked 5 inline comments as done.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54919.
guoyunhe added a comment.


  Output to stderr

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54918=54919

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html
  vue.min.js

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54918.
guoyunhe added a comment.


  Don't use remote JavaScript library

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54688=54918

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html
  vue.min.js

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Pino Toscano]

pino added a comment.


  In D19812#436174 , @guoyunhe wrote:
  
  > In D19812#436154 , @pino wrote:
  >
  > > - please harden the script using at least -e and -u flags for set: this 
way, it will not keep executing when a command fails, and undeclared variables 
are not silently expanded to empty string (to prevent typos)
  >
  >
  > Sorry, I am not very familiar with shell script. What does the -e or -u 
flags mean and how to use them? Can you give an example? Thanks.
  
  
  
  
  - -e: exits whenever any of the programs return a non-zero (i.e. failure) 
return code; this is useful to not silently ignore failures, and makes the 
behaviour similar to each line in a target of a Makefile
  - -u: immediately fails when trying to expand a variable that was not 
previously set; this way, things like `mkdir "$DIR/foo"` will immediately fail 
if `$FOO` was not set previously (so prevent misbehaviours due to typos, or 
code path not taken into account)

INLINE COMMENTS

> generate_web_data.sh:7
> +then
> +echo "Error: Directory $DIR/icons does not exist."
> +exit

this should go to stderr, as it is an error

> generate_web_data.sh:13
> +then
> +echo "Error: Directory $DIR/icons-dark does not exist."
> +exit

ditto

> index.html:93
> +  
> +  https://unpkg.com/vue";>
> +

Always using the network is not exactly a good idea:

- the page is unusable if there is no Internet connection
- this (private!) website will be phoned home every time an user loads this 
page locally, without even notifying the user

At least in Debian I see a `libjs-vue` package, so please make sure to work 
with local copies only. Otherwise this is a big privacy concern.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54688.
guoyunhe added a comment.


  Fix typo

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54546=54688

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> generate_web_data.sh:7
> +then
> +echo "Error: Directory $DIR/icons does not exists."
> +exit

exists -> exist

> generate_web_data.sh:13
> +then
> +echo "Error: Directory $DIR/icons-dark does not exists."
> +exit

ditto

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54546.
guoyunhe added a comment.


  Improve bash script

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54535=54546

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#436154 , @pino wrote:
  
  > - please harden the script using at least -e and -u flags for set: this 
way, it will not keep executing when a command fails, and undeclared variables 
are not silently expanded to empty string (to prevent typos)
  
  
  Sorry, I am not very familiar with shell script. What does the -e or -u flags 
mean and how to use them? Can you give an example? Thanks.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Pino Toscano]

pino added a comment.


  Few notes on the new `generate_web_data.sh` script:
  
  - please quote the paths properly, otherwise it will break when either the 
source directory or the build directory contain e.g. spaces
  - it does not seem using any bash-specific features, so make it using 
`/bin/sh`, to help non-Linux Unices (where usually bash is not available by 
default, and/or not in `/bin`)
  - please harden the script using at least -e and -u flags for set: this way, 
it will not keep executing when a command fails, and undeclared variables are 
not silently expanded to empty string (to prevent typos)

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54535.
guoyunhe added a comment.


  Add search function

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54533=54535

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  F6709477: image.png 

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54533.
guoyunhe added a comment.


  Add color blind mode

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54532=54533

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54532.
guoyunhe added a comment.


  Move build files to CMake build directory

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54257=54532

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  They should all go into the build dir IMO.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#434089 , @ngraham wrote:
  
  > Thanks, it compiles with CMake now! \o/
  >
  > I'm not a fan of how it dumps all this stuff in the source directory. 
Ideally it would put everything to the build directory when doing an 
out-of-source build so the source dir isn't cluttered up with transient and 
rapidly-changing information. Then you also wouldn't need to add these files to 
`gitignore`.
  
  
  The challenge is that in HTML file, it uses certain relative paths to icons 
and data files. CMake build directory can be anywhere. I have to make sure 
icons, index.html, .breeze.txt always in same relative paths.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  Thanks, it compiles with CMake now! \o/
  
  I'm not a fan of how it dumps all this stuff in the source directory. Ideally 
it would put everything to the build directory when doing an out-of-source 
build so the source dir isn't cluttered up with transient and rapidly-changing 
information. Then you also wouldn't need to add these files to `gitignore`.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Noah Davis]

ndavis added a comment.


  In D19812#433941 , @guoyunhe wrote:
  
  > In D19812#433916 , @ndavis wrote:
  >
  > > Could you use `#eff0f1` and `#31363b` for the light and dark backgrounds 
since those are the colors we normally use for window backgrounds?
  >
  >
  > Like this?
  >
  > F6700542: image.png 
  
  
  Yes

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54257.
guoyunhe added a comment.


  Use typical Breeze background colors

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54252=54257

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#433916 , @ndavis wrote:
  
  > Could you use `#eff0f1` and `#31363b` for the light and dark backgrounds 
since those are the colors we normally use for window backgrounds?
  
  
  Like this?
  
  F6700542: image.png 

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Noah Davis]

ndavis added a comment.


  Could you use `#eff0f1` and `#31363b` for the light and dark backgrounds 
since those are the colors we normally use for window backgrounds?

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#432424 , @ngraham wrote:
  
  > Getting closer:
  >
  >   Scanning dependencies of target generate-web
  >   make[2]: generate_web_data.sh: Command not found
  >   CMakeFiles/generate-web.dir/build.make:57: recipe for target 
'CMakeFiles/generate-web' failed
  >
  >
  > Are you able to test this yourself?
  
  
  Now it should work. I forgot to add `${CMAKE_SOURCE_DIR}` before the script 
name.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54252.
guoyunhe added a comment.


  Add ${CMAKE_SOURCE_DIR} prefix to script

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54051=54252

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  Getting closer:
  
Scanning dependencies of target generate-web
make[2]: generate_web_data.sh: Command not found
CMakeFiles/generate-web.dir/build.make:57: recipe for target 
'CMakeFiles/generate-web' failed
  
  Are you able to test this yourself?

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54051.
guoyunhe added a comment.


  Move outside of generate_binary_resource

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54050=54051

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#432348 , @bcooksley 
wrote:
  
  > I assume this fetches the icons from your local system?
  
  
  yes. it depends on text files generated by scripts, which contains list of 
all icons

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  mmm, I'm afraid that didn't work. running `cmake` on it reveals the following:
  
CMake Error at CMakeLists.txt:71 (add_custom_target):
  add_custom_target cannot create target "generate-web" because another
  target with the same name already exists.  The existing target is a custom
  target created in source directory "/home/dev/kde/src/breeze-icons/icons".
  See documentation for policy CMP0002 for more details.
  
  You have it inside the `generate_binary_resource` function, which means that 
it can be called twice. You'll need to move this target outside of that 
function.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Ben Cooksley]

bcooksley added a comment.


  I assume this fetches the icons from your local system?

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54050.
guoyunhe added a comment.


  Remove unnecessary spaces

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54049=54050

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#432326 , @ngraham wrote:
  
  > Neato! Looks pretty handy. Two issues I found:
  >
  > - generate_web_data.sh isn't executable
  > - I'd like to see this run as part of the build process, so that 
`index.html` gets updated automatically, and so running it doesn't pollute your 
source folder since you're of course doing out-of-source builds :) Then you 
don't have to add the generated text files to .gitignore.
  
  
  Hi @ngraham , I have added execution bit to the script. I have too little 
knowledge of CMake. Not sure if this will do what you said. Can you give some 
hints?

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54049.
guoyunhe added a comment.


  Add CMake build steps for web page

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54048=54049

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  CMakeLists.txt
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham, #vdg, ndavis
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe edited the summary of this revision.
guoyunhe added a reviewer: Breeze.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added reviewers: VDG, ndavis.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe updated this revision to Diff 54048.
guoyunhe added a comment.


  Fix data updating issues

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19812?vs=54047=54048

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  generate_web_data.sh
  index.html

To: guoyunhe, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Neato! Looks pretty handy. Two issues I found:
  
  - generate_web_data.sh isn't executable
  - I'd like to see this run as part of the build process, so that `index.html` 
gets updated automatically, and so running it doesn't pollute your source 
folder since you're of course doing out-of-source builds :) Then you don't have 
to add the generated text files to .gitignore.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
guoyunhe requested review of this revision.

REVISION SUMMARY
  This simple web page provides a table of all icons. So developers can easily 
know which icon size is
  missing, or not in dark theme.

REPOSITORY
  R266 Breeze Icons

BRANCH
  master

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

AFFECTED FILES
  .gitignore
  generate_web_data.sh
  index.html

To: guoyunhe
Cc: kde-frameworks-devel, michaelh, ngraham, bruns