WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 6 months ago

#6531 assigned defect (bug)

Recursively search for files in theme and plugin editors

Reported by: torbens Owned by: chsxf
Milestone: Future Release Priority: normal
Severity: minor Version: 2.5
Component: Themes Keywords: has-patch, dev-feedback, theme-editor
Focuses: administration Cc:

Description

Themes (like Subtle: http://gluedideas.com/downloads/subtle/) might contain numerous CSS files. The theme editor, however, does not recognize any other CSS files other than style.css.

The files might be located some levels deeper in sub directories like '/wp-content/themes/glued-ideas-subtle-01/assets/css/print.css'.

Attachments (7)

patch-recursive-editors.diff (10.5 KB) - added by chsxf 4 years ago.
24050.diff (2.4 KB) - added by MikeHansenMe 2 years ago.
patch from 24050 that was a duplicate of this ticket.
24050.2.diff (2.4 KB) - added by MikeHansenMe 2 years ago.
should fix the bug mentioned in comment 19
6531.diff (3.6 KB) - added by MikeHansenMe 18 months ago.
Show path for plugins and themes
6531.2.diff (3.7 KB) - added by MikeHansenMe 18 months ago.
Does not show if description and relative file are the same
6531.3.diff (7.6 KB) - added by valendesigns 7 months ago.
6531.4.diff (7.6 KB) - added by valendesigns 7 months ago.

Download all attachments as: .zip

Change History (37)

comment:1 @lloydbudd7 years ago

  • Milestone changed from 2.5.1 to 2.6

comment:2 @mrmist7 years ago

  • Keywords needs-patch added

Could be lumped in with #6632 maybe. No point in having 2 generic editors. Not sure how common buried stylesheets are.

comment:3 @mrmist7 years ago

See also #4131. This is possibly a dupe.

comment:4 @Denis-de-Bernardy6 years ago

  • Type changed from defect (bug) to enhancement

It's not common, but it occurs. The patch from #6632 partially sorts this if the files are in a direct subfolder. If you've a file any deeper, it won't do any good. example:

wp-content/themes/sem-reloaded/skins/copywriter-blue/skin.css

comment:5 @Denis-de-Bernardy6 years ago

  • Summary changed from CSS files are not shown in Theme Editor to Recursively search for files in theme and plugin editors

the same may also apply to plugins.

comment:6 @Denis-de-Bernardy6 years ago

  • Milestone changed from 2.9 to Future Release

comment:7 @chsxf4 years ago

  • Cc christophe@… added
  • Keywords has-patch needs-testing added; needs-patch removed
  • Owner changed from anonymous to chsxf
  • Status changed from new to assigned

Patch is attached.

It includes :

  • recursive search for files in both Theme and Plugin editors
  • sorting of files by path and name
  • inclusion of JavaScript files in the Theme editor

comment:8 @hm2k4 years ago

  • Cc hm2k added
  • Type changed from enhancement to defect (bug)

Is there any logical reason why you would not display all of the theme's CSS files from subdirectories in the theme editor?

Changing to a bug because expected behaviour is that all CSS files in the theme are shown.

Version 0, edited 4 years ago by hm2k (next)

comment:9 @chsxf4 years ago

The patch i've submitted two months ago brings this fonctionnality for CSS, JS and PHP files.

comment:10 @hm2k4 years ago

Is there any reason why you would NOT want to apply this patch?

Is there any reason you would NOT want this feature?

This bug has been open for 4 years, is there any reason why this bug cannot be closed?

Thanks!

comment:11 @WraithKenny4 years ago

  • Cc Ken@… added

comment:12 @prionkor3 years ago

  • Type changed from defect (bug) to enhancement

I would like to see this on future release. Perhaps the directory structure with js/jq implement.

comment:14 @SergeyBiryukov2 years ago

#23500 was marked as a duplicate.

comment:16 @SergeyBiryukov2 years ago

#24050 was marked as a duplicate.

@MikeHansenMe2 years ago

patch from 24050 that was a duplicate of this ticket.

comment:17 @Daedalon2 years ago

  • Cc daedalon@… added
  • Type changed from enhancement to defect (bug)

Thanks Mike for the patch! Verified, works. Also improves the coding style to match http://make.wordpress.org/core/handbook/coding-standards/php/.

I'd like to see this included in 3.6 after two improvements:

  1. Use curly braces with all ifs. Avoids hassle and bugs in later edits. A decent percentage of bugs are caused by editing code like this:
} else {
	if ( plugin_basename("$dir/$file") != $plugin )
		$plugin_files[] = plugin_basename("$dir/$file");
}
  1. Theme editor could use the same functionality.

Marking ticket as defect (bug) instead of enhancement. More appropriate for the file list not displaying all files of a plugin or theme. Milestone, anyone?

comment:18 @Daedalon2 years ago

  1. In order to improve performance, instead of calling this function twice inside a loop:
    plugin_basename("$dir/$file")
    

Split it into these parts, the first of which is to be called only once in the beginning of the function:

$plugin_basedir = plugin_basename($dir);
...
"$plugin_basedir/$file"

The patch in #24049 also applies this change.

comment:19 @Daedalon2 years ago

Encountered a bug:

  1. After opening a file from within a subdirectory, the Plugin Files lists only files and directories under that subdirectory.

@MikeHansenMe2 years ago

should fix the bug mentioned in comment 19

comment:20 @MikeHansenMe2 years ago

  • Cc mdhansen@… added

comment:21 @MikeHansenMe2 years ago

Turns out the bug in comment 19 affects core now. Should it be a separate ticket, even though 24050.2.diff fixes it? Also I still think there needs to be some ui improvements for the long list.

Last edited 2 years ago by MikeHansenMe (previous) (diff)

comment:22 @a.hoereth2 years ago

  • Cc a.hoereth@… added

comment:23 @MikeHansenMe2 years ago

  • Keywords needs-ui added

I think if we want to get this into core we need to get a UI that supports a long list of files and file tree structure.

comment:24 @helen18 months ago

  • Keywords needs-patch added; has-patch needs-testing needs-ui removed

Let's not let this ticket get hung up on UI - see #24048 for that instead.

As for this, what if we go with displaying the path for some clarity in themes, I guess the way plugins do now? And could we get a unified patch addressing all of the above? Can't apply the existing ones together, but one only deals with plugins :)

Last edited 18 months ago by helen (previous) (diff)

comment:25 @Daedalon18 months ago

Agreed with Helen. It's crucial to be able to see all the files in subfolders, and secondary to prettify it. Making all of the files visible is also pretty much a prerequisite for making the list neat. The sooner we display the full list in any ugly way, the sooner we can start to work on the chrome and polish for it.

Regarding Mike's excellent patches above, my opinion is that if there's one patch that fixes all of them for both plugins and themes, that'd be just perfect.

@MikeHansenMe18 months ago

Show path for plugins and themes

comment:26 @MikeHansenMe18 months ago

  • Keywords has-patch added; needs-patch removed

@MikeHansenMe18 months ago

Does not show if description and relative file are the same

comment:27 @nacin18 months ago

  • Component changed from Template to Themes
  • Focuses administration added

@valendesigns7 months ago

comment:28 @valendesigns7 months ago

  • Keywords dev-feedback added

The 6531.3.diff patch includes unit tests to prove that get_plugin_files will now return paths beyond two directory levels deep. This new patch subtly updates the UI for both plugins and themes. The previous patch didn't change the UI for plugins, but did for themes. I've updated the code to improve performance according to comment #18. I also solved an issue where the select dropdown was not showing the correct selected plugin when editing a file that wasn't the main plugin file.

C'mon Guys. Let's get this ticket closed! It's been open for 7 years.

@valendesigns7 months ago

comment:29 @valendesigns7 months ago

I guess I didn't test the theme code thoroughly enough from the previous patch. When I was showing someone how to apply and create a patch I noticed a bug when viewing the tree with a non default theme. Some of the files had empty strings in the nonessential span element which meant you just saw "()" under the file without text inside. I've fixed it in the latest patch.

Last edited 7 months ago by valendesigns (previous) (diff)

comment:30 @obenland6 months ago

  • Keywords theme-editor added
Note: See TracTickets for help on using tickets.