From e627190ac81914a28f91d91734622f6651520509 Mon Sep 17 00:00:00 2001 From: Sam Davies Date: Tue, 28 Jul 2020 13:35:30 +0100 Subject: [PATCH 1/3] WEB-3513: Parsing errors in chapter metadata are now labelled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't verify the metadata, but we do now add some location info so that it's possible to tell which file has the problem—rather than having to search through every single chapter to find the extra space in the indentation... --- app/lib/linting/linter.rb | 21 +++++++++++++++++++-- app/lib/parser/error.rb | 17 +++++++++++++++++ app/lib/parser/markdown_metadata.rb | 8 +++++++- 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 app/lib/parser/error.rb diff --git a/app/lib/linting/linter.rb b/app/lib/linting/linter.rb index d88de75..61630f2 100644 --- a/app/lib/linting/linter.rb +++ b/app/lib/linting/linter.rb @@ -20,7 +20,7 @@ def lint(options: {}) output end - def lint_with_ui(options:, show_ui: true) # rubocop:disable Metrics/MethodLength + def lint_with_ui(options:, show_ui: true) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize with_spinner(title: 'Checking {{bold:publish.yaml}} exists', show: show_ui) do check_publish_file_exists end @@ -31,6 +31,11 @@ def lint_with_ui(options:, show_ui: true) # rubocop:disable Metrics/MethodLength end return unless annotations.blank? + with_spinner(title: 'Attempting to parse book', show: show_ui) do + book + end + return unless annotations.blank? + with_spinner(title: 'Validating image references', show: show_ui) do annotations.concat(Linting::ImageLinter.new(book: book).lint) end @@ -78,11 +83,23 @@ def check_publish_file_exists false end - def book + def book # rubocop:disable Metrics/MethodLength @book ||= begin parser = Parser::Publish.new(file: file) parser.parse end + rescue Parser::Error => e + line_number = (e.message.match(/at line (\d+)/)&.captures&.first&.to_i || 0) + 1 + annotations.push( + Annotation.new( + absolute_path: e.file, + annotation_level: 'failure', + start_line: line_number, + end_line: line_number, + message: e.message, + title: 'Unable to parse book.' + ) + ) end end end diff --git a/app/lib/parser/error.rb b/app/lib/parser/error.rb new file mode 100644 index 0000000..d6e66be --- /dev/null +++ b/app/lib/parser/error.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Parser + # An error for capturing parsing issues + class Error < StandardError + attr_reader :file + + def initialize(file:, error: nil, msg: nil) + message = "There was a problem parsing #{file}" + message += "\n#{error.class.name}\n#{error.message}" if error.present? + message += "\n#{msg}" if msg.present? + + super(message) + @file = file + end + end +end diff --git a/app/lib/parser/markdown_metadata.rb b/app/lib/parser/markdown_metadata.rb index aa7ef55..84b1f38 100644 --- a/app/lib/parser/markdown_metadata.rb +++ b/app/lib/parser/markdown_metadata.rb @@ -12,7 +12,13 @@ def extract_metadata end def metadata - @metadata ||= Psych.load(extract_metadata).deep_symbolize_keys + @metadata ||= begin + Psych.load(extract_metadata, symbolize_names: true).tap do |metadata| + raise Parser::Error.new(file: path, msg: 'Unable to locate metadata at the top of the markdown') unless metadata.present? + end + end + rescue Psych::SyntaxError => e + raise Parser::Error.new(file: path, error: e) end def simple_attributes From b60f6b26f114d8dd188d136f1e8ba8353e4e0c33 Mon Sep 17 00:00:00 2001 From: Sam Davies Date: Tue, 28 Jul 2020 18:10:35 +0100 Subject: [PATCH 2/3] WEB-3514: Adding a framework for linting markdown content This didn't actually solve the problem I was trying to solve, but I think it might be useful in future, so I'll leave it in. --- app/lib/linting/image/attachable.rb | 2 +- app/lib/linting/linter.rb | 4 ++++ app/lib/linting/markdown/book.rb | 23 +++++++++++++++++++++++ app/lib/linting/markdown/chapter.rb | 20 ++++++++++++++++++++ app/lib/linting/markdown/renderable.rb | 25 +++++++++++++++++++++++++ app/lib/linting/markdown/section.rb | 23 +++++++++++++++++++++++ app/lib/linting/markdown_linter.rb | 16 ++++++++++++++++ 7 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 app/lib/linting/markdown/book.rb create mode 100644 app/lib/linting/markdown/chapter.rb create mode 100644 app/lib/linting/markdown/renderable.rb create mode 100644 app/lib/linting/markdown/section.rb create mode 100644 app/lib/linting/markdown_linter.rb diff --git a/app/lib/linting/image/attachable.rb b/app/lib/linting/image/attachable.rb index 0df957c..cf017b0 100644 --- a/app/lib/linting/image/attachable.rb +++ b/app/lib/linting/image/attachable.rb @@ -20,7 +20,7 @@ def lint_attachments end end - def locate_errors(image) + def locate_errors(_image) [{ start_line: 1, end_line: 1 diff --git a/app/lib/linting/linter.rb b/app/lib/linting/linter.rb index 61630f2..2dd0bf6 100644 --- a/app/lib/linting/linter.rb +++ b/app/lib/linting/linter.rb @@ -36,6 +36,10 @@ def lint_with_ui(options:, show_ui: true) # rubocop:disable Metrics/MethodLength end return unless annotations.blank? + with_spinner(title: 'Validating markdown', show: show_ui) do + annotations.concat(Linting::MarkdownLinter.new(book: book).lint) + end + with_spinner(title: 'Validating image references', show: show_ui) do annotations.concat(Linting::ImageLinter.new(book: book).lint) end diff --git a/app/lib/linting/markdown/book.rb b/app/lib/linting/markdown/book.rb new file mode 100644 index 0000000..a519773 --- /dev/null +++ b/app/lib/linting/markdown/book.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Linting + module Markdown + # Lint the markdown associated with a book + class Book + include Linting::Markdown::Renderable + + def self.lint(book) + new(book).lint + end + + def lint + [].tap do |annotations| + annotations.concat(lint_markdown_attributes) + object.sections.each do |section| + annotations.concat(Linting::Markdown::Section.lint(section)) + end + end + end + end + end +end diff --git a/app/lib/linting/markdown/chapter.rb b/app/lib/linting/markdown/chapter.rb new file mode 100644 index 0000000..745a402 --- /dev/null +++ b/app/lib/linting/markdown/chapter.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Linting + module Markdown + # Lint the markdown associated with a chapter + class Chapter + include Linting::Markdown::Renderable + + def self.lint(chapter) + new(chapter).lint + end + + def lint + [].tap do |annotations| + annotations.concat(lint_markdown_attributes) + end + end + end + end +end diff --git a/app/lib/linting/markdown/renderable.rb b/app/lib/linting/markdown/renderable.rb new file mode 100644 index 0000000..a79bc16 --- /dev/null +++ b/app/lib/linting/markdown/renderable.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Linting + module Markdown + # Check that the attributes marked as markdown renderable are valid + module Renderable + include Linting::FileExistenceChecker + + attr_reader :object + + def initialize(object) + @object = object + end + + def lint_markdown_attributes + object.markdown_render_loop do |content, is_file| + markdown = is_file ? File.read(content) : content + # TODO: Do some checking here + content + end + [] + end + end + end +end diff --git a/app/lib/linting/markdown/section.rb b/app/lib/linting/markdown/section.rb new file mode 100644 index 0000000..659d156 --- /dev/null +++ b/app/lib/linting/markdown/section.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Linting + module Markdown + # Lint the markdown associated with a section + class Section + include Linting::Markdown::Renderable + + def self.lint(section) + new(section).lint + end + + def lint + [].tap do |annotations| + annotations.concat(lint_markdown_attributes) + object.chapters.each do |chapter| + annotations.concat(Linting::Markdown::Chapter.lint(chapter)) + end + end + end + end + end +end diff --git a/app/lib/linting/markdown_linter.rb b/app/lib/linting/markdown_linter.rb new file mode 100644 index 0000000..cf59ba5 --- /dev/null +++ b/app/lib/linting/markdown_linter.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Linting + # Lints the markdown in a book + class MarkdownLinter + attr_reader :book + + def initialize(book:) + @book = book + end + + def lint + Linting::Markdown::Book.lint(book) + end + end +end From 67109569c9d5c7569f69f9a9c449fceab1bdccbe Mon Sep 17 00:00:00 2001 From: Sam Davies Date: Sun, 9 Aug 2020 19:40:56 +0100 Subject: [PATCH 3/3] Attempting to make full semver docker tags --- .github/workflows/build-docker.yml | 32 +++++++++++++++++++----------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build-docker.yml b/.github/workflows/build-docker.yml index 26ad40a..7022ff2 100644 --- a/.github/workflows/build-docker.yml +++ b/.github/workflows/build-docker.yml @@ -84,17 +84,21 @@ jobs: VERSION=$(echo "${{ github.ref }}" | sed -e 's,.*/\(.*\),\1,') # Prefix tags with 'release' - [[ "${{ github.ref }}" == "refs/tags/"* ]] && VERSION=release-$VERSION + if [[ "${{ github.ref }}" == "refs/tags/"* ]]; then + TAGS=$(echo "${VERSION}" | sed -E "s/v?([0-9]+)\.([0-9]+)\.([0-9]+)/release-\1.\2.\3 release-\1.\2 release-\1/g") + fi # Use Docker `latest` tag convention - [ "$VERSION" == "master" ] && VERSION=latest - [ "$VERSION" == "development" ] && VERSION=staging + [ "$VERSION" == "master" ] && TAGS=latest + [ "$VERSION" == "development" ] && TAGS=staging echo IMAGE_ID=$IMAGE_ID - echo VERSION=$VERSION + echo TAGS=$TAGS - docker tag image $IMAGE_ID:$VERSION - docker push $IMAGE_ID:$VERSION + for TAG in ${TAGS}; do + docker tag image "${IMAGE_ID}:${TAG}" + docker push "${IMAGE_ID}:${TAG}" + done - name: Push image to GitHub run: | @@ -107,17 +111,21 @@ jobs: VERSION=$(echo "${{ github.ref }}" | sed -e 's,.*/\(.*\),\1,') # Prefix tags with 'release' - [[ "${{ github.ref }}" == "refs/tags/"* ]] && VERSION=release-$VERSION + if [[ "${{ github.ref }}" == "refs/tags/"* ]]; then + TAGS=$(echo "${VERSION}" | sed -E "s/v?([0-9]+)\.([0-9]+)\.([0-9]+)/release-\1.\2.\3 release-\1.\2 release-\1/g") + fi # Use Docker `latest` tag convention - [ "$VERSION" == "master" ] && VERSION=latest - [ "$VERSION" == "development" ] && VERSION=staging + [ "$VERSION" == "master" ] && TAGS=latest + [ "$VERSION" == "development" ] && TAGS=staging echo IMAGE_ID=$IMAGE_ID - echo VERSION=$VERSION + echo TAGS=$TAGS - docker tag image $IMAGE_ID:$VERSION - docker push $IMAGE_ID:$VERSION + for TAG in ${TAGS}; do + docker tag image "${IMAGE_ID}:${TAG}" + docker push "${IMAGE_ID}:${TAG}" + done - name: Logout of DockerHub Registry if: ${{ always() }}