Code Formatter Vs 2022

0 views
Skip to first unread message

Mary Hargrove

unread,
Aug 4, 2024, 10:52:50 PM8/4/24
to monzenfrodi
Notethat this issue generalizes beyond the particular example of SQL. Any code that is written with any vertical formatting, particularly tabular constructs, is susceptible to destruction by a pretty printer.

I'm looking for something like a special comment that toggles the Eclipse formatter. Ideally, such a comment could be configurable to be whatever we choose, and other formatters could be programmed to respect it as well:


Obviously, one "solution" is to have our team members standardize on some external formatter like Jalopy or JIndent, but that's not what this question is about (also, not my decision on this project): I'm specifically looking for a way to avoid the Eclipse formatter on an ad-hoc basis.


There is another solution that you can use to suppress the formatting of specific block comments. Use /*- (note the hyphen) at the beginning of the block comment, and the formatting won't be affected if you format the rest of the file.


I'm using fixed width string-parts (padded with whitespace) to avoid having the formatter mess up my SQL string indentation. This gives you mixed results, and won't work where whitespace is not ignored as it is in SQL, but can be helpful.


Alternative method: In Eclipse 3.6, under "Line Wrapping" then "General Settings" there is an option to "Never join already wrapped lines." This means the formatter will wrap long lines but not undo any wrapping you already have.


This would have prevented it from happening in the first place since your coworkers are reformatting code they didn't actually change. This is a good practice to prevent mishaps that render diff on your source control useless (when an entire file is reformatted because of minor format setting differences).


Actually, you probably DO want formatting right? You want to remove mixed tab/space and trailing spaces. You want to indent the lines according to the code standard. What you DONT want is a long line. That, and only that, is what the phantom comment gives you!


I would suggest not to use the formatter. Bad code should look bad not artificially good. Good code takes time. You cannot cheat on quality. Formatting is part of source code quality.


I am trying to format an entire repo using a code formatter tool. In doing so, I want to keep information about who committed which line, so that commands like git blame still show the correct information. By this, I mean it should show the author that previously edited each line (before it was formatted).


It will take forever to run this and really I don't care about the past. I just want to format the master branch going forward without changing ownership of each line. How can I do this? I tried playing with the rev-list at the end and other filter types but it still doesn't work. There must be a way to format the codebase while preserving the author information for each line.


What you are trying to do is impossible. You cannot, at some point in time, change a line of code, and yet have git report that the most recent change to that line of code is something that happened before that point in time.


I suppose a source control tool could support the idea of an "unimportant change", where you mark a commit as cosmetic and then history analysis would skip over that commit. I'm not sure how the tool would verify that the change really was cosmetic, and without some form of tool enforcement the feature would assuredly be misused resulting in bug introductions potentially being hidden in "unimportant" commits. But really the reasons I think it's a bad idea are academic here - the bottom line is, git doesn't have such a feature. (Nor can I think of any source control tool that does.)


You can change the formatting going forward. You can preserve the visibility of past changes. You can avoid editing history. But you cannot do all three at the same time, so you're going to have to decide which one to sacrifice.


As you've noted, the straightforward way to do this with filter-branch would be very time consuming. There are things you can do to speed it up (like giving it a ramdisk for its working tree), but it's a tree-filter and it involves processing of each version of each file.


If you did some pre-processing, you could be somewhat more efficient. For example, you might be able to preprocess every BLOB in the database and create a mapping (where a TREE contains BLOB X, replace it with BLOB Y), and then use an index-filter to perform the substitutions. This would avoid all the checkout and add operations, and it would avoid repeatedly re-formatting the same code files. So that saves a lot of I/O. But it's a non-trivial thing to set up, and still might be time consuming.


Even if you come to a solution that will run fast enough, bear in mind that the history rewrite will disturb all of your refs. Like any history rewrite, it will be necessary for all users of the repo to update their clones - and for something this sweeping, the way I recommend to do that is, throw the clones out before you start the rewrite and re-clone afterward.


That also means if you have anything that depends on commit ID's, that will also be broken. (That could include build infrastructure, or release documentation, etc.; depending on your project's practices.)


So, a history rewrite is a pretty drastic solution. And on the other hand, it also seems drastic to suppose that formatting the code is impossible simply because it wasn't done from day 1. So my advice:


Do the reformatting in a new commit. If you need to use git blame, and it points you to the commit where reformatting occurred, then follow up by running git blame again on the reformat commit's parent.


One thing you could do is to branch from some earlier commit, reformat the code, and then rebase master to your branch. That would preserve authorship for all the changes that came after whatever commit you start from.


Rebasing a shared branch is a bad idea. The fact that you even care about preserving the authorship of changes probably means that there are a number of people actively working on the code. If you go and rebase the master branch, then every fork or clone of your repo is going to have a master branch with the old history, and that's bound to cause confusion and pain unless you're very careful about managing the process and making certain that everybody is aware of what you're doing and updates their copies appropriately. A better approach would probably be to not rebase master, but instead merge the commits from master into your branch. Then, have everybody start using the new branch instead of master.


Merge conflicts. In reformatting the entire codebase, you're probably going to make changes to a large number of lines in almost every file. When you merge the subsequent commits, whether that's via rebase or merge, you'll likely have a large number of conflicts to resolve. If you take the approach I suggested above and merge commits from master into your new branch instead of rebasing, then it'll be easier to resolve those conflicts in an orderly way because you can merge a few commits at a time until you're caught up.


Incomplete solution. You're going to have to figure out where in the history you want to insert your reformatting operation. The farther back you go, the more you'll preserve the authorship of changes, but the more work it'll be to merge in the subsequent changes. So you'll probably still end up with lots of code where your reformatting commit is the latest change.


Limited benefit. You never actually lose authorship information in git -- it's just that tools typically only show who made the most recent change. But you can still go back and look at prior commits and dig through the entire history of any piece of code, including who made it. So the only thing that inserting your reformatting operation into the history really buys you is the convenience of seeing who changed some piece of code without the extra step of going back to an earlier commit.


It's dishonest. When you rewrite the history of a branch, you're changing a factual recording of how the code changed over time, and that can create real problems. Let's imagine that your reformatting isn't quite as inconsequential as you mean it to be, and in doing the reformatting you actually create a bug. Let's say, for example, that you introduce some extra white space into a multi-line string constant. Weeks later, somebody finally notices the problem and goes looking for the cause, and it looks like the change was made a year and a half ago (because that's where you inserted your reformatting into the history). But the problem seems new -- it doesn't show up in the build that shipped two months ago, so what the heck is going on?


Benefit diminishes over time. As development continues, the changes that you're trying to hard not to cover up will be covered up by some other changes anyway, and your reformatting changes would likewise be superseded by those new changes. As time and development march on, the work you do to bury your reformatting changes won't mean much.


If you don't want your name showing up as the author of every line in your project, but you also don't want to live with the problems described above, then you might want to rethink you approach. A better solution might be to tackle the reformatting as a team: get everyone on the team to agree to run the formatter on any file that they change, and make proper formatting a requirement in all code reviews going forward. Over time, your team will cover most of the code, and the authorship information will be mostly appropriate since every file that gets reformatted was going to be changed anyway. You may eventually end up with a small number of files that never get reformatted because they're very stable and don't need updates, and you can choose to reformat them (because having some badly formatted files makes you nuts) or not (because nobody is really working in those files anyway).

3a8082e126
Reply all
Reply to author
Forward
0 new messages