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(check-line-alignment): avoid adding whitespace if hyphen at end of line #986

Merged
merged 1 commit into from
Feb 11, 2023

Conversation

brettz9
Copy link
Collaborator

@brettz9 brettz9 commented Feb 11, 2023

fixes #983

@brettz9 brettz9 added the bug label Feb 11, 2023
@brettz9 brettz9 merged commit f420121 into gajus:main Feb 11, 2023
@github-actions
Copy link

🎉 This PR is included in version 39.9.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@brettz9 brettz9 deleted the postHyphen-eol branch February 11, 2023 15:08
@thernstig
Copy link

thernstig commented Feb 12, 2023

@brettz9 thank you for looking into this.

I tested this and the bug fix did not work. Did you test it and it removed the white-space for you?

Here is another example where it does not complain about the white-space at the end of line when it should (notice the - ):

  /**
   * Flat selectedFiles to an array.
   *
   * @returns {{ nodeType: string, startDate: Date, fileName: string }[]} - 
   * list of selected files, ascending sorted by startDate
   */

@brettz9
Copy link
Collaborator Author

brettz9 commented Feb 12, 2023

The issue you reported that this fixes, and which this PR adds a test case for, is that a terminal hyphen will no longer get whitespace added to it.

What you are mentioning now is an opposite issue where whitespace is already present and you want it removed. I guess we could handle that too, but that is really a separate issue.

@thernstig
Copy link

thernstig commented Feb 12, 2023

@brettz9 my issue was about that it added a whitespace.

We had this at 39.6.4:

   * @param {string|string[]|TemplateResult|TemplateResult[]} event.detail.description -
   *    Notification description

It turned it into this at 39.8.0 when running --fix:

   * @param {string|string[]|TemplateResult|TemplateResult[]} event.detail.description - 
   *    Notification description

I also tried 39.9.1 and using --fix again made no change, it made it still look like the last one above with the added white space.

The rationale for reporting this is that it breaks Prettier and many other linters/formatters that remove spaces at end of line.

EDIT:

Or I see now what is going on here. I did the upgrade to 39.8.0 and applied the --fix. It added a whitespace after the terminal hyphen.

I then upgraded to 39.9.1 and ran --fix and it never removed the hyphen.

So in a sense yes, you are absolutely correct there. In my unlucky streak of upgrades I ended up with a trailing whitespace. So I suppose removing a trailing whitespace should also happen then possibly. Up to you of course.

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

Successfully merging this pull request may close these issues.

check-line-alignment with option postHyphen adds white space at end of line
2 participants