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

Fix formatting of comments above import declarations #41481

Merged

Conversation

poorna2152
Copy link
Contributor

@poorna2152 poorna2152 commented Oct 6, 2023

Purpose

Describe the problems, issues, or needs driving this feature/fix and include links to related issues.

Fixes #41478
Fixes #41613

Approach

Describe how you are implementing the solutions along with the design details.

Samples

Provide high-level details about the samples related to this feature.

Remarks

List any other known issues, related PRs, TODO items, or any other notes related to the PR.

Check List

  • Read the Contributing Guide
  • Updated Change Log
  • Checked Tooling Support (#)
  • Added necessary tests
    • Unit Tests
    • Spec Conformance Tests
    • Integration Tests
    • Ballerina By Example Tests
  • Increased Test Coverage
  • Added necessary documentation
    • API documentation
    • Module documentation in Module.md files
    • Ballerina By Examples

@poorna2152 poorna2152 requested a review from hevayo as a code owner October 6, 2023 11:32
@poorna2152 poorna2152 added Team/CompilerFETools Semantic API, Formatter, Shell Area/Formatting labels Oct 6, 2023
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Attention: Patch coverage is 94.23077% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 76.64%. Comparing base (09e0471) to head (3f2b4d6).
Report is 8 commits behind head on master.

Files Patch % Lines
...g/ballerinalang/formatter/core/FormatterUtils.java 92.30% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #41481       +/-   ##
=============================================
+ Coverage      0.00%   76.64%   +76.64%     
- Complexity        0    53691    +53691     
=============================================
  Files             9     2907     +2898     
  Lines            35   203057   +203022     
  Branches          0    26470    +26470     
=============================================
+ Hits              0   155624   +155624     
- Misses           35    38934    +38899     
- Partials          0     8499     +8499     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

AzeemMuzammil
AzeemMuzammil previously approved these changes Oct 31, 2023
Copy link
Member

@AzeemMuzammil AzeemMuzammil left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

Copy link

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

Copy link

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

Copy link

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

Copy link

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

@poorna2152 poorna2152 force-pushed the comments_before_imports branch 2 times, most recently from e98deca to c5b626a Compare April 1, 2024 02:46
@MaryamZi
Copy link
Member

MaryamZi commented Apr 1, 2024

  1. Formatting
// Copyright (c) 2023 WSO2 LLC. (http://www.wso2.org) All Rights Reserved.
//
// WSO2 LLC. licenses this file to you under the Apache License,
// Version 2.0 (the "License"); you may not use this file except
// in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied.  See the License for the
// specific language governing permissions and limitations
// under the License.

// check this
import ballerinax/twilio as _;
import ballerina/jballerina.java as _;

adds an empty line between // check this and the first import. Is this intentional? I don't think that's required.

  1. [Blocker] After I formatted the above I replaced the file content with just
int i = 1;

and formatted the file (all via VSCode). This removed my changes and added

// Copyright (c) 2023 WSO2 LLC. (http://www.wso2.org) All Rights Reserved.
//
// WSO2 LLC. licenses this file to you under the Apache License,
// Version 2.0 (the "License"); you may not use this file except
// in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied.  See the License for the
// specific language governing permissions and limitations
// under the License.

// check this

import ballerina/jballerina.java as _;
import ballerinax/twilio as _;

@poorna2152
Copy link
Contributor Author

  1. Formatting

A newline is added separating the top most comments and the top most import node. Thus this is the expected behaviour.

  1. [Blocker] After I formatted the above I replaced the file content with just

This might not be relevant to this change and this is may be relevant to this

@poorna2152 poorna2152 force-pushed the comments_before_imports branch from ed8c29d to ff84e47 Compare April 4, 2024 02:09
@poorna2152 poorna2152 force-pushed the comments_before_imports branch 2 times, most recently from 304c25d to 9ff8588 Compare April 4, 2024 02:15
@poorna2152 poorna2152 force-pushed the comments_before_imports branch from 9ff8588 to f45f13c Compare April 4, 2024 02:29
@MaryamZi MaryamZi added this to the 2201.9.0 milestone Apr 4, 2024
@MaryamZi MaryamZi merged commit 68c91b5 into ballerina-platform:master Apr 4, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Formatting Team/CompilerFETools Semantic API, Formatter, Shell
Projects
None yet
6 participants