WIP: feature_paste #1

Draft
Miroslavsckaya wants to merge 24 commits from feature_paste into master
8 changed files with 218 additions and 9 deletions
Showing only changes of commit 918872fde1 - Show all commits

View file

@ -10,6 +10,7 @@
"doctrine/doctrine-bundle": "^2.10",
"doctrine/doctrine-migrations-bundle": "^3.2",
"doctrine/orm": "^2.15",
"scrivo/highlight.php": "v9.18.1.10",
Review

Why so specific?

Why so specific?
"symfony/console": "6.3.*",
"symfony/dotenv": "6.3.*",
"symfony/flex": "^2",

80
composer.lock generated
View file

@ -4,7 +4,7 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
"This file is @generated automatically"
],
"content-hash": "f716d81393aa0f266916ea52e0fa0b53",
"content-hash": "de2f472bb7e2d9dbd3c6363258ac286e",
"packages": [
{
"name": "doctrine/cache",
@ -1521,6 +1521,84 @@
},
"time": "2021-07-14T16:46:02+00:00"
},
{
"name": "scrivo/highlight.php",
"version": "v9.18.1.10",
"source": {
"type": "git",
"url": "https://github.com/scrivo/highlight.php.git",
"reference": "850f4b44697a2552e892ffe71490ba2733c2fc6e"
},
"dist": {
"type": "zip",
"url": "https://api.github.com/repos/scrivo/highlight.php/zipball/850f4b44697a2552e892ffe71490ba2733c2fc6e",
"reference": "850f4b44697a2552e892ffe71490ba2733c2fc6e",
"shasum": ""
},
"require": {
"ext-json": "*",
"php": ">=5.4"
},
"require-dev": {
"phpunit/phpunit": "^4.8|^5.7",
"sabberworm/php-css-parser": "^8.3",
"symfony/finder": "^2.8|^3.4|^5.4",
"symfony/var-dumper": "^2.8|^3.4|^5.4"
},
"suggest": {
"ext-mbstring": "Allows highlighting code with unicode characters and supports language with unicode keywords"
},
"type": "library",
"autoload": {
"files": [
"HighlightUtilities/functions.php"
],
"psr-0": {
"Highlight\\": "",
"HighlightUtilities\\": ""
}
},
"notification-url": "https://packagist.org/downloads/",
"license": [
"BSD-3-Clause"
],
"authors": [
{
"name": "Geert Bergman",
"homepage": "http://www.scrivo.org/",
"role": "Project Author"
},
{
"name": "Vladimir Jimenez",
"homepage": "https://allejo.io",
"role": "Maintainer"
},
{
"name": "Martin Folkers",
"homepage": "https://twobrain.io",
"role": "Contributor"
}
],
"description": "Server side syntax highlighter that supports 185 languages. It's a PHP port of highlight.js",
"keywords": [
"code",
"highlight",
"highlight.js",
"highlight.php",
"syntax"
],
"support": {
"issues": "https://github.com/scrivo/highlight.php/issues",
"source": "https://github.com/scrivo/highlight.php"
},
"funding": [
{
"url": "https://github.com/allejo",
"type": "github"
}
],
"time": "2022-12-17T21:53:22+00:00"
},
{
"name": "symfony/cache",
"version": "v6.3.1",

74
public/css/ocean.css Normal file
View file

@ -0,0 +1,74 @@
/* Ocean Dark Theme */
/* https://github.com/gavsiu */
/* Original theme - https://github.com/chriskempson/base16 */
/* Ocean Comment */
.hljs-comment,
.hljs-quote {
color: #65737e;
}
/* Ocean Red */
.hljs-variable,
.hljs-template-variable,
.hljs-tag,
.hljs-name,
.hljs-selector-id,
.hljs-selector-class,
.hljs-regexp,
.hljs-deletion {
color: #bf616a;
}
/* Ocean Orange */
.hljs-number,
.hljs-built_in,
.hljs-builtin-name,
.hljs-literal,
.hljs-type,
.hljs-params,
.hljs-meta,
.hljs-link {
color: #d08770;
}
/* Ocean Yellow */
.hljs-attribute {
color: #ebcb8b;
}
/* Ocean Green */
.hljs-string,
.hljs-symbol,
.hljs-bullet,
.hljs-addition {
color: #a3be8c;
}
/* Ocean Blue */
.hljs-title,
.hljs-section {
color: #8fa1b3;
}
/* Ocean Purple */
.hljs-keyword,
.hljs-selector-tag {
color: #b48ead;
}
.hljs {
display: block;
overflow-x: auto;
background: #2b303b;
color: #c0c5ce;
padding: 0.5em;
}
.hljs-emphasis {
font-style: italic;
}
.hljs-strong {
font-weight: bold;
}

View file

@ -4,6 +4,7 @@ declare(strict_types = 1);
namespace App\Controller;
use App\DTO\PasteFormData;
use App\Entity\CodeHighlighter;
use App\Entity\Paste;
use App\Form\Type\PasteForm;
use App\Repository\PasteRepository;
@ -43,8 +44,9 @@ class PasteController extends AbstractController
$pasteData = new PasteFormData($paste);
$form = $this->createForm(PasteForm::class, $pasteData);
skobkin marked this conversation as resolved Outdated

You're violating Symfony code style here which as I can guess you used above.

You can just set Symfony style in the settings of your IDE for this project and auto-format this file to fix. Or fix it manually if you want.

You're violating [Symfony code style](https://symfony.com/doc/current/contributing/code/standards.html#symfony-coding-standards-in-detail) here which as I can guess you used above. You can just set Symfony style in the settings of your IDE for this project and auto-format this file to fix. Or fix it manually if you want.
skobkin marked this conversation as resolved Outdated

Do you need double quotes here too?

Do you need double quotes here too?
return $this->render('paste.html.twig', [
return $this->render('show_paste.html.twig', [
'form' => $form,
'highlighted_text' => CodeHighlighter::highlight($pasteData->language, $pasteData->text)
Review

I'd suggest you implementing Twig extension wih filter and function to use highlighter in the template.

I'd suggest you implementing Twig extension wih filter and function to use highlighter in the template.
]);
}
}

View file

@ -0,0 +1,26 @@
<?php
declare(strict_types = 1);
namespace App\Entity;
Review

Why Entity?
Are you storing it in the database? Does it have an identity?

Why `Entity`? Are you storing it in the database? Does it have an identity?
use \Highlight\Highlighter;
class CodeHighlighter
{
public static function highlight(?string $language, string $code): string
{
$hl = new Highlighter();
Review

Why not inject into constructor?

Why not inject into constructor?
try {
// Highlight some code.
$highlighted = $hl->highlight($language, $code);
$highlighted_text = "<pre><code class=\"hljs {$highlighted->language}\">".$highlighted->value.'</code></pre>';
Review

Why do you use HTML here and not in the template? Check the comment I left in PasterController::showPaste().

Why do you use HTML here and not in the template? Check the comment I left in `PasterController::showPaste()`.
Review

Also naming here is not good. You have $highlighted and $highlighted_text.

  • $highlighted_text is not in $camelCase
  • Both variables contain text, both contain code. If it shouldn't be removed, I'd say to think a bit more on naming.
Also naming here is not good. You have `$highlighted` and `$highlighted_text`. - `$highlighted_text` is not in `$camelCase` - Both variables contain text, both contain code. If it shouldn't be removed, I'd say to think a bit more on naming.
}
catch (\DomainException $e) {
// This is thrown if the specified language does not exist
$highlighted_text = '<pre><code>'.htmlentities($code).'</code></pre>';
Review

Again why HTML here?

Again why HTML here?
}
return $highlighted_text;
}
}

View file

@ -2,15 +2,14 @@
<html>
<head>
<meta charset="UTF-8">
<title>{% block title %}Welcome!{% endblock %}</title>
<title>{% block title %}Copypaste{% endblock %}</title>
<link rel="icon" href="data:image/svg+xml,<svg xmlns=%22http://www.w3.org/2000/svg%22 viewBox=%220 0 128 128%22><text y=%221.2em%22 font-size=%2296%22>⚫️</text></svg>">
<link href="https://cdn.jsdelivr.net/npm/bootstrap@5.0.2/dist/css/bootstrap.min.css" rel="stylesheet" integrity="sha384-EVSTQN3/azprG1Anm3QDgpJLIm9Nao0Yz1ztcQTwFspd3yD65VohhpuuCOmLASjC" crossorigin="anonymous">
Review

It's better not to rely on external CDN's when you can.
You can just save needed resources in the repository since we're not making our front-end too complex to make dynamic build.

It's better not to rely on external CDN's when you can. You can just save needed resources in the repository since we're not making our front-end too complex to make dynamic build.
{% block stylesheets %}
{% endblock %}
{% block javascripts %}
{% endblock %}
</head>
<body>
{% block body %}{% endblock %}
{% block content %}{% endblock %}
<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.0.2/dist/js/bootstrap.bundle.min.js" integrity="sha384-MrcW6ZMFYlzcLA8Nl+NtUVF0sA7MsXsP1UyJoMp4YLEuNSfAP+JcXn/tWtIaxVXM" crossorigin="anonymous"></script>
Review

Same here.

Same here.
</body>
</html>

View file

@ -1 +1,6 @@
{% extends "base.html.twig" %}
skobkin marked this conversation as resolved Outdated

Did you forget to extend base.html.twig and use it's body block?

Did you forget to extend `base.html.twig` and use it's `body` block?
Review

Why double quotes again?

Why double quotes again?
Review

BTW, if you're including it in the show_paste.html.twig, then you don't need to extend base.html.twig here.

BTW, if you're including it in the `show_paste.html.twig`, then you don't need to extend `base.html.twig` here.
{% block content %}
{{ form(form) }}
{% endblock %}

View file

@ -0,0 +1,24 @@
{% extends "base.html.twig" %}
{% block stylesheets %}
<link rel="stylesheet" href="/css/ocean.css"></link>
{% endblock %}
{% block content %}
<ul class="nav nav-tabs" id="myTab" role="tablist">
Review

Do not forget to name classes and ID's properly.

Do not forget to name classes and ID's properly.
<li class="nav-item" role="presentation">
<button class="nav-link active" id="show-tab" data-bs-toggle="tab" data-bs-target="#show" type="button" role="tab" aria-controls="show" aria-selected="true">Show</button>
</li>
<li class="nav-item" role="presentation">
<button class="nav-link" id="profile-tab" data-bs-toggle="tab" data-bs-target="#edit" type="button" role="tab" aria-controls="edit" aria-selected="false">Edit</button>
Review

profile-tab?

`profile-tab`?
</li>
</ul>
<div class="tab-content" id="myTabContent">
<div class="tab-pane fade show active" id="show" role="tabpanel" aria-labelledby="show-tab">
{{ highlighted_text|raw }}
Review

As I said above, better implement Twig extension and use it here.

As I said above, better implement Twig extension and use it here.
</div>
<div class="tab-pane fade" id="edit" role="tabpanel" aria-labelledby="edit-tab">
{% include 'paste.html.twig' %}
</div>
</div>
{% endblock %}