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

Use blank lines instead of indentation as issue body markers #163

Open
firedev opened this issue Jul 12, 2021 · 12 comments
Open

Use blank lines instead of indentation as issue body markers #163

firedev opened this issue Jul 12, 2021 · 12 comments

Comments

@firedev
Copy link

firedev commented Jul 12, 2021

Indented lines after the first comment are being removed by automatic code for matters like standardrb and rubocop.

Here is a proposal. White space and indentation is not reliable. Treat non-interrupted comment blocks starting with an issue marker as the issue body. Until the first empty line or the beginning of the next code block:

# TODO: #1 Issue title
#
# ^ if you need an empty line for aesthetics
#
# Everything here is added as
# an issue body
#
# ... still the issue body

# This comment is left in the code
def hello

Note: The body should not start with the blank line in this example. Like with git commit the resulting body should start with the first meaningful line.

# TODO #1 This has only title
#

Originally posted by @firedev in #96 (comment)

@yegor256
Copy link
Member

@mbao01 WDYT?

@mbao01
Copy link
Contributor

mbao01 commented Jul 12, 2021

Thanks so much, @firedev. I think it is a good idea, currently, this behaviour works following our marker formatting (i.e space after the beginning of the todo marker.

This works ✅

# TODO: #1 Issue title
#  <- invisible whitespaces here
#  ^ if you need an empty line for aesthetics
#  <- invisible whitespaces here
#  Everything here is added as
#  an issue body
#  <- invisible whitespaces here
#  ... still the issue body

Output:

Issue title ^ if you need an empty line for aesthetics Everything here is added as an issue body ... still the issue body

This proposal will require us to rework how we parse next-line content for a puzzle. It looks good @yegor256
Blank comment lines part of the contiguous comment block can be treated as a new line in the puzzle text.

Desired Output:

Issue title
^if you need an empty line for aesthetics
Everything here is added as an issue body
... still the issue body

What do you think @firedev?

@firedev
Copy link
Author

firedev commented Jul 12, 2021

When you convert a block of code to a ticket you need to add spaces manually, you can't just slap a todo market and move on as you normally would:

# TODO #1 Refactor the following
# xspecify "failing test" do
#   here too? or keep 2 spaces.
#    oops, 3 spaces here, whatever
# end

I believe "significant whitespace in comments" is a brittle concept. I personally strip all extra whitespace from code. All double spaces, and double empty lines are replaces with a single newline or space. Maybe this is why I had issues adding todos.

If you use blank line or the beginning of code as end markers the process becomes frictionless. You work as you've used to and pdd enhances your workflow without hindering.

@firedev
Copy link
Author

firedev commented Jul 14, 2021

@yegor256 I am trying to implement this and actually went through the code and fixed all todos. And then instructed my teammates to add a blank line after # TODO. The parser chokes on everything and I don't want to spend time formatting comments.

@0pdd is not creating issues yet though yegor256/0pdd#304

@mbao01
Copy link
Contributor

mbao01 commented Sep 15, 2021

@yegor256 please see comment above

@yegor256
Copy link
Member

@mbao01 which one exactly?

@mbao01
Copy link
Contributor

mbao01 commented Nov 3, 2021

@yegor256 I want to confirm if we are going in this direction for puzzle markers. Based on the thread, is this something we feel confident about?

@yegor256
Copy link
Member

@mbao01 I think that the idea is good. Let's try to implement it and see how our tests will react to it. If there won't be a lot of damage to them, let's release.

mbao01 added a commit to mbao01/pdd that referenced this issue Jan 3, 2022
mbao01 added a commit to mbao01/pdd that referenced this issue Jan 3, 2022
mbao01 added a commit to mbao01/pdd that referenced this issue Feb 7, 2022
@mbao01
Copy link
Contributor

mbao01 commented Feb 11, 2022

@firedev This has been fixed in #187. It will be available in the next release.
Please can you close this issue if everything looks good?

@php-coder
Copy link
Contributor

php-coder commented Jul 9, 2022

Guys, please, could you introduce such changes in backward-compatible format? It requires users to update their code base. Sometimes, it's impossible and, anyway, how users of pdd/0pdd would know that such changes will happen soon? When 0pdd gets updated, we just have a new version and only because something is broken, I found that there was such a change :-(

It seems like, at least couple of the newly created duplicated issues are caused by this change. See yegor256/0pdd#366

@yegor256
Copy link
Member

@firedev @mbao01 what do you think?

@firedev
Copy link
Author

firedev commented Jul 11, 2022

@php-coder @yegor256
I would vote for the simplest logic for todo extraction. And I also vote for automatic code beautification.

I couldn't make it work with the double space after the marker. I don't see my editors formatting text like this when commenting code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants