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

[#224] Add description for multiline puzzle #237

Merged
merged 12 commits into from Feb 4, 2024
2 changes: 1 addition & 1 deletion Gemfile
Expand Up @@ -27,7 +27,7 @@ gem 'minitest', '5.16.2', require: false
gem 'rake', '13.0.6', require: false
gem 'rdoc', '6.4.0', require: false
gem 'rspec-rails', '5.1.2', require: false
gem 'rubocop', '1.53.1', require: false
gem 'rubocop', '1.60.2', require: false
gem 'rubocop-rspec', '2.22.0', require: false
gem 'simplecov', '0.22.0', require: false
gem 'slop', '4.9.2', require: false
Expand Down
43 changes: 36 additions & 7 deletions README.md
Expand Up @@ -146,6 +146,10 @@ Example:
* @todo #234:15m/DEV This is something to do later
* in one of the next releases. I can't figure out
* how to implement it now, that's why the puzzle.
* The text can be so long, as needed, just use
* the same anount of spaces, as the second line.
* This text will be not a part of the puzzle, as
* it has less spaces.
*/
void sendEmail() {
throw new UnsupportedOperationException();
Expand All @@ -172,13 +176,11 @@ as long as you have one of the 3 supported keywords right in front
of the mandatory marker):

```
// @todo #224
/* @todo #TEST-13 */
Copy link
Contributor

Choose a reason for hiding this comment

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

Why it was removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not supported in the code, there is only a logic to remove prefix, but no logic to remove any kinds of symbols from the end

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, I'd suggest to fix/improve support for such type of comments instead of removing it from documentation.

Copy link
Contributor Author

@pnatashap pnatashap Jan 27, 2024

Choose a reason for hiding this comment

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

There is a bug about it #225
I think it is better to remove it now and return it back after fix

Choose a reason for hiding this comment

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

@pnatashap If we remove this examples, I believe we have to remove TEST-13 and 42 from the next sentence too:

224, TEST-13, 55, 67, 678, 1, and 42 are the IDs of the tickets these puzzles are coming from.

# @todo #55:45min
@todo #67/DES
;; @todo #678:40m/DEV
// TODO: #1:30min
(* TODO #42 *)
Copy link
Contributor

Choose a reason for hiding this comment

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

The same: why it has been removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not supported in the code, there is only a logic to remove prefix, but no logic to remove any kinds of symbols from the end

// @todo #224 Puzzle description
# @todo #55:45min Puzzle description
@todo #67/DES Puzzle description
;; @todo #678:40m/DEV Puzzle description
// TODO: #1:30min Puzzle description
```

Here `DES` and `DEV` are the roles of people who must fix that puzzles;
Expand All @@ -192,6 +194,33 @@ us to build a hierarchical dependency tree of all puzzles, like
for example. Technically, of course, you can abuse the system
and put a dummy `#1` marker everywhere.

### Multiline examples

```xml
<!--
~ if comment should be started and closed by special symbols, then place them in

Choose a reason for hiding this comment

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

@pnatashap What Is "prefix" here? ~ ? I haven't found the definition of "prefix" in the README file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be used to decorate text, not required, but for there is no restrictions about the prefix, it should be just equal for all text. So it is just an example

Choose a reason for hiding this comment

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

@pnatashap, maybe we can add this to the documentation? I mean, we can state that the "prefix" is a symbol that can be used to decorate text; it isn't required. And provide some examples.

Moreover, I would suggest adding some headers or small - one-sentence descriptions to these code examples:

  1. "...Prefix definition... Here we ignore the prefix, and only one puzzle will be created."
  2. "We can also use the \ symbol to connect multiline puzzles."

What do you think?

~ a separate lines without any text
~ Any symbol can be used as a prefix, it will be excluded from the text
~
~ @todo #34 Description can not be created in one line with comment
~ symbols. Just use at least the same amount of the spaces, as on the first line.
-->
```

```java
/**
* @todo #36 Multiline text can use the same prefix in all lines or the same
* amount of spaces.
* So this will be added to the puzzle description. If you want to divide the
* puzzle by empty line, just add a backspace to that line
* \
* and continue the text after.
*
* This line is not part of the puzzle, because the line before contains less
* spaces then the second line.
*/
```

## How to Configure Rules?

You can specify post-parsing rules for your puzzles, in command line,
Expand Down
20 changes: 0 additions & 20 deletions features/step_definitions/steps.rb
Expand Up @@ -83,10 +83,6 @@
raise "STDOUT doesn't contain '#{txt}':\n#{@stdout}" unless @stdout.include?(txt)
end

Then(/^Stdout is empty$/) do

Choose a reason for hiding this comment

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

@pnatashap, I don't understand why you decided to remove some steps from steps.rb. The report states the following:

28 scenarios (23 undefined, 5 passed)
138 steps (34 skipped, 28 undefined, 76 passed)

However, we used to have:

28 scenarios (28 passed)
138 steps (138 passed)

Maybe it's worth keeping all the scenarios passing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Locally it was displayed, that it is duplicated steps and should be removed. Forget that they do not fail. Returned them back

Copy link
Contributor Author

@pnatashap pnatashap Feb 1, 2024

Choose a reason for hiding this comment

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

A flag is added to cucumber run to fail the task in case of undefined step

raise "STDOUT is not empty:\n#{@stdout}" unless @stdout == ''
end

Then(/^XML file "([^"]+)" matches "([^"]+)"$/) do |file, xpath|
raise "File #{file} doesn't exit" unless File.exist?(file)

Expand All @@ -103,14 +99,6 @@
if content.index(substring).nil?
end

Then(/^Exit code is zero$/) do
raise "Non-zero exit code #{@exitstatus}" unless @exitstatus.zero?
end

Then(/^Exit code is not zero$/) do
raise 'Zero exit code' if @exitstatus.zero?
end

When(/^I run bash with$/) do |text|
FileUtils.copy_entry(@cwd, File.join(@dir, 'pdd'))
@stdout = `#{text}`
Expand All @@ -122,11 +110,3 @@
@stdout = `#{text}`
@exitstatus = $CHILD_STATUS.exitstatus
end

Given(/^It is Unix$/) do
pending if Gem.win_platform?
end

Given(/^It is Windows$/) do
pending unless Gem.win_platform?
end
36 changes: 30 additions & 6 deletions lib/pdd/source.rb
Expand Up @@ -144,13 +144,37 @@ def minutes(num, units)
def tail(lines, prefix, start)
return [] if lines.empty?
prefix = " #{' ' * start}" if prefix.empty? # fallback to space indentation
empty_prefix = ' ' * (prefix.length + 1)
prefix = "#{prefix} " if lines[0][prefix.length, 1]&.start_with?(' ')
lines
.take_while do |t|
match_markers(t).none? && (t.start_with?(prefix) || t.start_with?(empty_prefix))
tail_prefix = puzzle_tail_prefix(lines, prefix)
tail = lines
.take_while { |t| puzzle_text?(t, tail_prefix, prefix) }
.map do |t|
content = t[tail_prefix.length, t.length]&.lstrip
puzzle_empty_line?(content, '') ? '' : content
end
tail.pop if tail[-1].eql?('')
tail
end

def puzzle_tail_prefix(lines, prefix)
return prefix if lines.empty?
i = 0
while i < lines.length
unless puzzle_empty_line?(lines[i], prefix)
return lines[i].start_with?("#{prefix} ") ? "#{prefix} " : prefix
end
.map { |t| t[prefix.length, t.length]&.lstrip }
i += 1
end
prefix
end

def puzzle_text?(line, prefix, intro_prefix)
return false unless match_markers(line).none?
line.start_with?(prefix) || puzzle_empty_line?(line, intro_prefix)
end

def puzzle_empty_line?(line, prefix)
return true if line.nil?
line.start_with?(prefix) && line.gsub(prefix, '').chomp.strip.eql?('\\')
end

# @todo #75:30min Let's make it possible to fetch Subversion data
Expand Down
2 changes: 1 addition & 1 deletion test/test_source.rb
Expand Up @@ -48,7 +48,7 @@ def test_parsing
assert_equal 2, list.size
puzzle = list.first
assert_equal '2-3', puzzle.props[:lines]
assert_equal 'привет, how are you doing?', \
assert_equal 'привет, how are you doing?',
puzzle.props[:body]
assert_equal '44', puzzle.props[:ticket]
assert puzzle.props[:author].nil?
Expand Down
63 changes: 57 additions & 6 deletions test/test_source_todo.rb
Expand Up @@ -112,6 +112,17 @@ def test_todo_colon_parsing
)
end

def test_todo_backslash_escape
check_valid_puzzle(
"
// TODO #45 task description with \\
",
'2-2',
'task description with \\',
'45'
)
end

def test_multiple_todo_colon
check_valid_puzzle(
"
Expand Down Expand Up @@ -150,22 +161,62 @@ def test_todo_colon_parsing_multi_line_with_empty_line
)
end

# space is needed in test data in the comment
# rubocop:disable Layout/TrailingWhitespace
def test_todo_colon_parsing_multi_line_with_empty_line_and_space
check_valid_puzzle(
'
// TODO: #46 task description
//
// second line after empty line is a part of the puzzle in case of space exists
// \
// second line after empty line is a part of the puzzle in case of backslash exists
',
'2-4',
'task description second line after empty line is a part ' \
'of the puzzle in case of space exists',
'of the puzzle in case of backslash exists',
'46'
)
end

def test_todo_colon_parsing_double_puzzle_with_empty_line
check_valid_puzzle(
'
// TODO: #46 task description for first
// \
// TODO: #47 task description
',
'2-2',
'task description for first',
'46',
2
)
end

def test_todo_parsing_puzzle_javadoc_with_empty_line
check_valid_puzzle(
'
/**
* TODO: #46 task description
* \
*/
* some text
',
'3-3',
'task description',
'46'
)
end
# rubocop:enable Layout/TrailingWhitespace

def test_todo_colon_parsing_multi_line_random_prefix
check_valid_puzzle(
'
~~
~~ @todo #44 First
~~ and
~~ second
',
'3-4',
'First and',
'44'
)
end

def test_todo_failing_no_ticket
check_invalid_puzzle(
Expand Down
2 changes: 1 addition & 1 deletion test_assets/puzzles/44-660e9d6f
Expand Up @@ -2,7 +2,7 @@
// @todo #44 This puzzle
// consists
// of
//
// \
// two
// paragraphs

Expand Down