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.

Sign up for GitHub

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

Jump to bottom

Fix regex of iconsCacher #10326

Merged
merged 4 commits into from
Jul 24, 2018
Merged

Fix regex of iconsCacher #10326

merged 4 commits into from
Jul 24, 2018

Conversation

skjnldsv
Copy link
Member

Fix #10309

@skjnldsv skjnldsv added bug 3. to review Waiting for reviews feature: caching Related to our caching system: scssCacher, jsCombiner... labels Jul 21, 2018
@skjnldsv skjnldsv added this to the Nextcloud 14 milestone Jul 21, 2018
@skjnldsv skjnldsv self-assigned this Jul 21, 2018
juliusknorr
juliusknorr reviewed Jul 21, 2018
View reviewed changes
lib/private/Template/IconsCacher.php Outdated
@@ -47,7 +47,7 @@ class IconsCacher {
protected $urlGenerator;

/** @var string */
private $iconVarRE = '/--(icon-[a-z0-9-]+): url\(["\']([a-z0-9-_\~\/\?\&\=\.]+)[^;]+;/m';
private $iconVarRE = '/--(icon-[a-z0-9-]+): url\(["\']([a-z0-9-_\~\/\.]+)[^;]+;/m';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just recognized it might make sense to also allow uppercase chars

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes I guess :)

@skjnldsv skjnldsv requested a review from juliusknorr July 21, 2018 18:19
rullzer
rullzer approved these changes Jul 21, 2018
View reviewed changes
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory fine. But I always like it is we have test for stuff like this.

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 22, 2018
@skjnldsv
Copy link
Member Author

Right! Let me add tests :)

@MorrisJobke
Copy link
Member

Now I get this instead:

[Error] Failed to load resource: the server responded with a status of 404 (Not Found) (000, line 0) 
http://localhost:8000/index.php/svg/core/files/star-dark/000?v=1

@MorrisJobke MorrisJobke mentioned this pull request Jul 24, 2018
Merged
21 tasks
@skjnldsv
Copy link
Member Author

@MorrisJobke wrong icon, star_dark comes from core/actions. I'll fix it

@MorrisJobke
Copy link
Member

Just noticed that it was before like this:

http://localhost:8000/index.php/svg/core/files/star-dark/000?v=2?v=1?v=1?v=1?v=1?v=1?v=1?v=1?v=1?v=1

skjnldsv added 4 commits July 24, 2018 12:08
@skjnldsv
Fix header controls sticky positioning
a9e554b 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Fix regex of iconsCacher
85a481d 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Uppercase
6bcfe8d 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
new tests
f62d4d1 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the svg-api-suf-fixes branch from 4702578 to f62d4d1 Compare July 24, 2018 10:21
@skjnldsv
Copy link
Member Author

Tests added!
@nextcloud/designers please review :)

@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 24, 2018
@skjnldsv
Copy link
Member Author

sh: 1: kill: No such process
Unrelated

@skjnldsv skjnldsv added the 4. to release Ready to be released and/or waiting for tests to finish label Jul 24, 2018
@skjnldsv skjnldsv removed the 3. to review Waiting for reviews label Jul 24, 2018
@MorrisJobke MorrisJobke merged commit f7c7ff5 into master Jul 24, 2018
@MorrisJobke MorrisJobke deleted the svg-api-suf-fixes branch July 24, 2018 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@rullzer rullzer rullzer approved these changes

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

Assignees

@skjnldsv skjnldsv

Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: caching Related to our caching system: scssCacher, jsCombiner...
Projects
None yet
Milestone
Nextcloud 14
Development

Successfully merging this pull request may close these issues.
4 participants
@skjnldsv @MorrisJobke @rullzer @juliusknorr