7 ms·

Not to mention it dirties up version control. It's a lot easier to tell which _significant_ things changed from the last commit when you're scanning a diff and

Not to mention it dirties up version control. It's a lot easier to tell which _significant_ things changed from the last commit when you're scanning a diff and no formatting changes are highlighted.

waheoo 6y ago
Ive seen heaps of autoformatters dirty up version control. Mostly when theyre fussing about new lines. I kind of wish autoformatters would just ignore new lines and focus on indentation alone. It would certainly cut down on the over zealous rules. I also think formatting should be separate from linting but because they both require a parser people seem to think theyre the same thing.
- SOLAR_FIELDS 6y ago
  Wouldn't having the autoformatter run as a pre-commit hook alleviate this issue? (This is assuming you had already cleaned up all of the currently checked in code with said autoformatter)
  - est31 6y ago
    Most autoformatters are under active development, and updates to them often causes changes in their behaviour. So you have to have all devs agree on the precise version of the formatter used, and even then you still need to do regular commits that contain nothing but a run of the newest version of the formatter on the codebase, which sadly destroys long term git blame.
    - cheez 6y ago
      this is a solved problem, pin your packages.
      - est31 6y ago
        But you still have to update them every now and then otherwise you can't use new features of your language. Which gives you the issue of breakages again.
        waheoo 6y ago
        I dont think people want to hear that there are still problems with autoformatting text. Anyway, my point still stands, drop rules about line length and rules that care about inter line break positions and most of these problems go away.
        cheez 6y ago
        Sure. Most large teams do this on a 6 month basis or when it is needed to implement something. Again, a solved problem.
    - zetafunction 6y ago
      Chromium has a presubmit that enforces that code is autoformatted. However, it only enforces this for changed lines, and in practice, it works pretty well.
  - waheoo 6y ago
    Prettier specifically annoys me to no end with this problem. If youre adding more attributes to a jsx component later and know it you might format to multiline early. It decides to undo your work and slow you down, because some arbitrary line length. Similarly, if you go over that line length, on something you want single line for readability of structure, it just straight up does what it wants. Im 100% on the auto format train but it has to go the gofmt route and only apply rules that are actually important and incredibly uncontroversial.
    - crooked-v 6y ago
      > It decides to undo your work What you're missing here is that you're supposed to be using it all along, all the time. Format on save. Format on commit. Format on server pipelines. Format all the time. It can never 'undo' anything if it's what's providing the formatting in the first place.
      - waheoo 6y ago
        Thats what im talking about. If you add text to a line, and it goes over a length, it will reformat a jsx element into about 5 lines instead of 1. Similarly, when you add more than it needs because you want consistency, or know youll add more, and the formatter runs, often it undoes your work.
        libria 6y ago
        It's tunable. Pick a line length you all agree on and when it reformats after that, that's what you asked for. If the rule never works for you, turn that one off. If the rule is what you want 99% of the time but it made this one line look dumb, mark that one block #ignore and let the team decide in CR if that was the right call.
        earthboundkid 6y ago
        If you really want 5 lines, add a dummy // in there and will be forced to use 5 instead of 1. Dirty hack, but it works.
- CGamesPlay 6y ago
  This problem is caused by adding a formatter to your editor, instead of adding it to your project (or one of your contributors adding it to their editor). But if you adopt a formatter into the project, then this isn't a problem. Create one commit called "reformat all files", and then use `--ignore-rev` to never see it in your git log. If your contributor is changing thousands of lines in a diff to fix a typo, ask them not to do that :)
- ornornor 6y ago
  My workaround is to run the formatted in the whole codebase as a separate PR, make the commit with a separate user that’s none of my colleagues (like “Prettier <invalid@acme.corp>”) and add that commit’s hash to a file in the repo. I then configure git to use that file for ignoring changes in git blame. Now the code is formatted and additions won’t trigger reformatting the line or the file.
gugagore 6y ago
That doesn't necessarily mean that you need to check in auto-formatted changes. You can just autoformat the two revisions before feeding it to the diff program. Of course, a lot of tooling simply isn't there for that, unfortunately, but that's what I see as the ideal solution.
mbaird 6y ago
We've found https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt https://git-scm.com/docs/git-blame#Documentation/git-blame.t... very useful in these cases. When Prettier changes significantly, we run across the whole codebase & add the commit to an ignore file.

Not to mention it dirties up version control. It's a lot easier to tell which _significant_ things changed from the last commit when you're scanning a diff and no formatting changes are highlighted.

waheoo 6y ago

Ive seen heaps of autoformatters dirty up version control. Mostly when theyre fussing about new lines. I kind of wish autoformatters would just ignore new lines and focus on indentation alone. It would certainly cut down on the over zealous rules. I also think formatting should be separate from linting but because they both require a parser people seem to think theyre the same thing.

SOLAR_FIELDS 6y ago
Wouldn't having the autoformatter run as a pre-commit hook alleviate this issue? (This is assuming you had already cleaned up all of the currently checked in code with said autoformatter)
- est31 6y ago
  Most autoformatters are under active development, and updates to them often causes changes in their behaviour. So you have to have all devs agree on the precise version of the formatter used, and even then you still need to do regular commits that contain nothing but a run of the newest version of the formatter on the codebase, which sadly destroys long term git blame.
  - cheez 6y ago
    this is a solved problem, pin your packages.
    - est31 6y ago
      But you still have to update them every now and then otherwise you can't use new features of your language. Which gives you the issue of breakages again.
      - waheoo 6y ago
        I dont think people want to hear that there are still problems with autoformatting text. Anyway, my point still stands, drop rules about line length and rules that care about inter line break positions and most of these problems go away.
      - cheez 6y ago
        Sure. Most large teams do this on a 6 month basis or when it is needed to implement something. Again, a solved problem.
  - zetafunction 6y ago
    Chromium has a presubmit that enforces that code is autoformatted. However, it only enforces this for changed lines, and in practice, it works pretty well.
- waheoo 6y ago
  Prettier specifically annoys me to no end with this problem. If youre adding more attributes to a jsx component later and know it you might format to multiline early. It decides to undo your work and slow you down, because some arbitrary line length. Similarly, if you go over that line length, on something you want single line for readability of structure, it just straight up does what it wants. Im 100% on the auto format train but it has to go the gofmt route and only apply rules that are actually important and incredibly uncontroversial.
  - crooked-v 6y ago
    > It decides to undo your work What you're missing here is that you're supposed to be using it all along, all the time. Format on save. Format on commit. Format on server pipelines. Format all the time. It can never 'undo' anything if it's what's providing the formatting in the first place.
    - waheoo 6y ago
      Thats what im talking about. If you add text to a line, and it goes over a length, it will reformat a jsx element into about 5 lines instead of 1. Similarly, when you add more than it needs because you want consistency, or know youll add more, and the formatter runs, often it undoes your work.
      - libria 6y ago
        It's tunable. Pick a line length you all agree on and when it reformats after that, that's what you asked for. If the rule never works for you, turn that one off. If the rule is what you want 99% of the time but it made this one line look dumb, mark that one block #ignore and let the team decide in CR if that was the right call.
      - earthboundkid 6y ago
        If you really want 5 lines, add a dummy // in there and will be forced to use 5 instead of 1. Dirty hack, but it works.
CGamesPlay 6y ago
This problem is caused by adding a formatter to your editor, instead of adding it to your project (or one of your contributors adding it to their editor). But if you adopt a formatter into the project, then this isn't a problem. Create one commit called "reformat all files", and then use `--ignore-rev` to never see it in your git log. If your contributor is changing thousands of lines in a diff to fix a typo, ask them not to do that :)
ornornor 6y ago
My workaround is to run the formatted in the whole codebase as a separate PR, make the commit with a separate user that’s none of my colleagues (like “Prettier <invalid@acme.corp>”) and add that commit’s hash to a file in the repo. I then configure git to use that file for ignoring changes in git blame. Now the code is formatted and additions won’t trigger reformatting the line or the file.

gugagore 6y ago

That doesn't necessarily mean that you need to check in auto-formatted changes. You can just autoformat the two revisions before feeding it to the diff program. Of course, a lot of tooling simply isn't there for that, unfortunately, but that's what I see as the ideal solution.

mbaird 6y ago

We've found https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt https://git-scm.com/docs/git-blame#Documentation/git-blame.t... very useful in these cases. When Prettier changes significantly, we run across the whole codebase & add the commit to an ignore file.