Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete Unused PNG Files #1580

Merged
merged 1 commit into from
Jan 29, 2025
Merged

Delete Unused PNG Files #1580

merged 1 commit into from
Jan 29, 2025

Conversation

BeckerWdf
Copy link
Contributor

No description provided.

@laeubi
Copy link
Contributor

laeubi commented Jan 24, 2025

@BeckerWdf it would be useful to tell in the PR description how you detected they are unused?

@BeckerWdf
Copy link
Contributor Author

@BeckerWdf it would be useful to tell in the PR description how you detected they are unused?

I did a search for the PNGs. If I found no usages in the PDE files then I assumed they are not used..

Copy link

github-actions bot commented Jan 24, 2025

Test Results

   285 files  ±0     285 suites  ±0   53m 33s ⏱️ + 2m 2s
 3 586 tests ±0   3 510 ✅ ±0   76 💤 ±0  0 ❌ ±0 
10 950 runs  ±0  10 719 ✅ ±0  231 💤 ±0  0 ❌ ±0 

Results for commit a5c398c. ± Comparison against base commit 7809ab4.

♻️ This comment has been updated with latest results.

@BeckerWdf
Copy link
Contributor Author

Anyway. I smoke test from a third person would be good to have.

@BeckerWdf
Copy link
Contributor Author

@vogella : Maybe you can review and double check this change too.

@vogella
Copy link
Contributor

vogella commented Jan 29, 2025

Note for myself:

# Find all files deleted by this commit: 
git diff --diff-filter=D --name-only HEAD^ HEAD | grep 'icons/' | sed 's|.*icons/|icons/|'
#get plug-in and file
git diff --diff-filter=D --name-only HEAD^ HEAD | awk -F'/' '{print $1 "/" $2, $NF}'

@vogella
Copy link
Contributor

vogella commented Jan 29, 2025

Impressive @BeckerWdf that you did this check manually. Only verifying your changes drives me almost crazy. I have added a little Dart script to check for possible reference so that I only need to check the remaining files reported by the script. As the full path is constructed in code, a full automatic check seems harder to implement.

If I pipe this output to the file "pngfiles.txt" I can search for the files with the following dart script:

git diff --diff-filter=D --name-only HEAD^ HEAD | awk -F'/' '/icons\// {print $1 "/" $2, $NF, $0}' >pngfiles.txt 
import 'dart:io';

void main() async {
  final inputFile = File('pngfiles.txt');

  final lines = await inputFile.readAsLines();

  final Set<String> foundFiles = {};
  final Map<String, String> foundPngFiles = {};

  for (var line in lines) {
    var parts = line.split(' ');

    var searchDir = Directory(parts[0]); // search from here
    var targetPng = parts[1];
    var fullOriginalPath = parts[2];

    if (await searchDir.exists()) {
      await for (var file
          in searchDir.list(recursive: true, followLinks: false)) {
        if (file is File &&
            (file.path.endsWith('.java') || file.path.endsWith('.xml'))) {
          var matches = await findMatchingLines(file, targetPng);
          if (matches.isNotEmpty) {
            foundFiles.add(file.path);
            foundPngFiles[targetPng] = fullOriginalPath;
            print('\n📄 File: ${file.path}');
            for (var match in matches) {
              print('   ➜ ${match.trim()}');
            }
          }
        }
      }
    }
  }

  if (foundFiles.isEmpty) {
    print('\nNo matching files found.');
  } else {
    print('\n🎯 Summary: PNG files found in Java/XML references:');
    for (var png in foundPngFiles.keys) {
      print('   🖼️ ${foundPngFiles[png]}');
    }
  }
}

Future<List<String>> findMatchingLines(File file, String text) async {
  List<String> matchingLines = [];
  try {
    var lines = await file.readAsLines();
    for (var line in lines) {
      if (line.contains(text)) {
        matchingLines.add(line);
      }
    }
  } catch (e) {
    print('Error reading file: ${file.path}');
  }
  return matchingLines;
}


Output for png files to check manually, potentially present with different paths:

🎯 Summary: PNG files found in Java/XML references:
   🖼️ ui/org.eclipse.pde.ui/icons/dlcl16/full_hierarchy.png
   🖼️ ui/org.eclipse.pde.ui/icons/dlcl16/maximize.png
   🖼️ ui/org.eclipse.pde.ui/icons/dlcl16/restore.png
   🖼️ ui/org.eclipse.pde.ui/icons/etool16/convjpprj_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/etool16/defcon_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/exp_deployfeat.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/exp_deployplug.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/exp_product.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/imp_extfeat.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/jarToPlugin.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/new_target_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newefix_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newex_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newexp_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newexprj_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newfprj_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newftrprj_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newpprj_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newsiteprj_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/prd_config_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/script_wiz.png
   🖼️ ui/org.eclipse.pde.ui/icons/dtool16/validate.png

I check until the first one which seems to have a reference: defcon_wiz private static final String PATH_TOOL = ICONS_PATH + "etool16/"; //$NON-NLS-1$

@BeckerWdf can you verify if that file is really not used?

@BeckerWdf
Copy link
Contributor Author

Thanks for checking

@BeckerWdf can you verify if that file is really not used?

Sure:
🖼️ ui/org.eclipse.pde.ui/icons/dlcl16/full_hierarchy.png --> Only the version in elcl16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dlcl16/maximize.png --> Only the version in elcl16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dlcl16/restore.png --> Only the version in elcl16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/etool16/convjpprj_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/etool16/defcon_wiz.png --> Used in unused constant -> Will delete constant. See next commit.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/exp_deployfeat.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/exp_deployplug.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/exp_product.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/imp_extfeat.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/jarToPlugin.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/new_target_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newefix_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newex_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newexp_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newexprj_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newfprj_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newftrprj_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newpprj_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/newsiteprj_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/prd_config_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/script_wiz.png --> Only the version in etool16 is used. This one is unused.
🖼️ ui/org.eclipse.pde.ui/icons/dtool16/validate.png --> Only the version in etool16 is used. This one is unused.

@vogella
Copy link
Contributor

vogella commented Jan 29, 2025

I also check the rest and with the removal of the constant this one looks good to me.

Thank a bunch @BeckerWdf for this work

@vogella vogella merged commit 2413246 into eclipse-pde:master Jan 29, 2025
18 checks passed
@BeckerWdf BeckerWdf deleted the unused_pngs branch January 29, 2025 14:12
@HannesWell
Copy link
Member

Thanks! Getting rid of unused resources is highly appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants