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

FormatRBear: Allow to disable parameter-options for formatR #514

Closed
Makman2 opened this issue Jun 13, 2016 · 16 comments · Fixed by #1335
Closed

FormatRBear: Allow to disable parameter-options for formatR #514

Makman2 opened this issue Jun 13, 2016 · 16 comments · Fixed by #1335

Comments

@Makman2
Copy link
Member

Makman2 commented Jun 13, 2016

Sometimes certain style rules shall not be checked at all, we should allow to disable them (like r_use_arrows).

CC @AsnelChristian

@gitmate-bot
Copy link
Collaborator

Thanks for reporting this issue!

Your aid is required, fellow coalaian. Help us triage and solving this issue!

CC @sils1297, @AbdealiJK

@AsnelChristian
Copy link
Member

can i work on this one ?

@sils
Copy link
Member

sils commented Jun 14, 2016

sure!

AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 15, 2016
The options: `r_keep_comments` and `r_use_arrows` passed to
to tidy_source, don't really need to be checked.
This commit disables `r_keep_comments` if it is `None`,
the same goes for `r_use_arrows`
Fixes coala#514
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 15, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 15, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 15, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 15, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 16, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 16, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 17, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 18, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 20, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 20, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 20, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 20, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 22, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 26, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 27, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 28, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jun 28, 2016
AsnelChristian added a commit to AsnelChristian/coala-bears that referenced this issue Jul 2, 2016
@Makman2
Copy link
Member Author

Makman2 commented Jul 2, 2016

Ah in the PR message this issue was referenced... but it's not solved yet

@Makman2 Makman2 reopened this Jul 2, 2016
@gs0510
Copy link
Contributor

gs0510 commented Oct 1, 2016

@Makman2 @sils1297 I see the issue is still open, but using r_use_arrows = true/false did exactly what it was supposed to do. Are there other style rules that should be allowed to be disabled?

@Makman2
Copy link
Member Author

Makman2 commented Oct 2, 2016

I think r_use_arrows = False (for example) forced to use no arrows at all. So it's either the one or other extreme when using such a setting. Though I could be wrong with this (long time ago when this issue was opened :3)

@gs0510
Copy link
Contributor

gs0510 commented Oct 2, 2016

Yes what you are saying is true. So is the requirement that in some places we should use the arrow and in other places we should still be using equal to sign? For example if we have r_use_arrows=true start_line_num=10 end_line_num=20, we can have arrows in line numbers from 10 to 20. Is that what is required? Can you please explain in a little detail what the requirements are? Thanks!

@Makman2
Copy link
Member Author

Makman2 commented Oct 3, 2016

If the setting is not touched inside a coafile, no check shall happen at all.

For example:

# inside coafile:
bears = FormatRBear
# some settings regarding FormatRBear except 'r_use_arrows',
# meaning we don't want to care whether '=' or '<-' is used.

This can be achieved via setting the default value for r_use_arrows to None (here -> https://github.com/coala/coala-bears/blob/master/bears/r/FormatRBear.py#L36) and omitting the according command-line-parameter if r_use_arrows = None (here -> https://github.com/coala/coala-bears/blob/master/bears/r/FormatRBear.py#L83).

(Though this setting is only allowed to be either True or False, coala doesn't check the default value, that's why None does not make any problem inside the declaration as a default value.)

@gs0510
Copy link
Contributor

gs0510 commented Oct 4, 2016

If I do not modify the r_use_arrows, and have both = and <- in my r file, nothing changes. The <- and = are both still intact. Here's an asciinema recording that I made. Hope this closes this issue.

@Makman2
Copy link
Member Author

Makman2 commented Oct 4, 2016

Oh than I was wrong :3
But does this work for all other parameters too? If so we can happily close this one indeed :)

(Btw it's interesting what patch is applied in your asciinema as I can't see any change in the file :3)

@gs0510
Copy link
Contributor

gs0510 commented Oct 4, 2016

Are you talking about Applied 'ApplyPatchAction' on 'test.r' from 'FormatRBear' ? I just did a diff before and after the change. The bear adds a newline at the end of the file.

@Makman2
Copy link
Member Author

Makman2 commented Oct 4, 2016

ah alright, nothing serious then :D

@yogupta
Copy link
Contributor

yogupta commented Jan 18, 2017

What else was needed in this ? I mean why is still open? Was anything missing?

@Makman2
Copy link
Member Author

Makman2 commented Jan 18, 2017

seems like it just wasn't continued^^

@yogupta
Copy link
Contributor

yogupta commented Jan 19, 2017

If I have understood the issue then we have to remove functionalities such as r_keep_comments , r_use_arrows. Do we have to remove r_braces_on_next_line ?
Is there anything else than i would like to know . I think i can solve this.

@Makman2
Copy link
Member Author

Makman2 commented Jan 19, 2017

No don't remove them, allow the bear to "ignore" the settings.

I think r_use_arrows = False (for example) forced to use no arrows at all. So it's either the one or other extreme when using such a setting.

And we want by default having sth like r_use_arrows = None and we just don't pass the argument to the R formatter. This applies for other settings as well, not only for r_use_arrows.

gosom pushed a commit to gosom/coala-bears that referenced this issue Jul 15, 2017
Default options of r_braces_on_next_line,
r_use_arrows are disabled.

Closes coala#514
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

6 participants