goldmansachs/reladomo

Tabs are there in generated sources as indentation

Closed this issue · 7 comments

KTONQ commented

Tabs are there in entity source generated by reladomo.
I found the indentation is hardcoded here, is it good to raise a pull request to change to spaces?
or is there any other approach to the matter?
Thanks.

case1: codeFormat = default value (fast)

here is the source of build.gradle:

task genReladomo {
    ant.taskdef(name: "genReladomo",
            classpath: configurations.reladomoGenTask.asPath,
            classname: "com.gs.fw.common.mithra.generator.MithraGenerator")
    ant.genReladomo(xml: "$projectDir/src/main/resources/reladomo/model/ReladomoClassList.xml",
            generateEcListMethod: "true",
            generatedDir: "$buildDir/generated-sources/reladomo",
            nonGeneratedDir: "$projectDir/src/main/java")
}

and here is one of the sources generated, with tabs as indentation.

public class ChannelList extends ChannelListAbstract
{
	public ChannelList()
	{
		super();
	}

	public ChannelList(int initialSize)
	{
		super(initialSize);
	}

	public ChannelList(Collection c)
	{
		super(c);
	}

	public ChannelList(Operation operation)
	{
		super(operation);
	}
}

case2: codeFormat = none

here is the source of build.gradle, with parameter codeFormat specified as "none".

task genReladomo {
    ant.taskdef(name: "genReladomo",
            classpath: configurations.reladomoGenTask.asPath,
            classname: "com.gs.fw.common.mithra.generator.MithraGenerator")
    ant.genReladomo(xml: "$projectDir/src/main/resources/reladomo/model/ReladomoClassList.xml",
            generateEcListMethod: "true",
            codeFormat: "none",
            generatedDir: "$buildDir/generated-sources/reladomo",
            nonGeneratedDir: "$projectDir/src/main/java")
}

indentation are gone, but 1 tab is there in another file generated, in the next line of "{".

public class ChannelDatabaseObject extends ChannelDatabaseObjectAbstract
{
	

}
mohrezaei commented

Tab or space is a preference, not a universal truth. If you change it to space, someone is going to come along and expect tabs.

For the non-concrete classes (those that get regenerated every time), it shouldn't matter what the spacing is.

For the concrete classes, you're expected to change the formatting (braces, method params, imports, etc), before you commit to your codebase. This is usually a single key press in an IDE.

motlin commented
KTONQ commented

For the non-concrete classes (those that get regenerated every time), it shouldn't matter what the spacing is.

Agree.

For the concrete classes, you're expected to change the formatting (braces, method params, imports, etc), before you commit to your codebase. This is usually a single key press in an IDE.

That's true,
but not every class need to be implemented.

I read this not as "spaces are better than tabs" but as "it's possible to get Reladomo to generate concrete classes with both spaces and tabs in the same file"

Correct.
Add one more parameter with default value as "tabs" maybe better choice.

mohrezaei commented

That's true,
but not every class need to be implemented.

I don't understand what you mean. Whether you have or don't have code in those classes, they must be committed to your source control. The person who first generates them (and commits them) is expected to format them according to your needs. If you've neglected to do this, you might want to fix this sooner rather than later. You can select all files you want to format in your IDE and reformat them with one command.

Add one more parameter with default value as "tabs" maybe better choice.

You make it sound like that's the only parameter that determines Java code formatting... It's not. Should we add parameters for:

  • braces
  • blank lines
  • method parameters per line
  • import style
  • end of line at end of file or not
  • ...

IDE's and Checkstyle have hundreds of such settings. Replicating that here seems wrong headed when your preference can be enforced with a single key press.

P.S. I don't think @KTONQ and @motlin mean the same thing with "it's possible to get Reladomo to generate concrete classes with both spaces and tabs in the same file"

KTONQ commented

Do you mean that,
Please format auto-generated sources yourself though they are being formatted as default value of codeFormat is "fast".

If understanding above is correct, it sounds like codeFormat itself is provided, but meaningless.
Of course it is unrealistic to meet all requirements, but is there any possibility to follow "common" basic coding style rules?

I thought you will agree at least for the indentation, since in the contributing guidelines, there are only 2 simple rules, and one of them is indentation.

P.S. I don't think @KTONQ and @motlin mean the same thing with "it's possible to get Reladomo to generate concrete classes with both spaces and tabs in the same file"

My mistake, I didn't mean "both spaces and tabs in the same file", sorry for confusing.

mohrezaei commented

The codeFormat setting is an old leftover from when we were trying different formatters. It's not really meant to represent the formatting options but just the implementation used.

I thought you will agree at least for the indentation, since in the contributing guidelines, there are only 2 simple rules, and one of them is indentation.

This is really not about what I or even Reladomo's internal coding style is about. I personally dislike tabs. This is about what makes sense for the project.

The concrete classes are generated as a courtesy once and they are expected to be fully owned by the project team. As such, any formatting is the responsibility of the owners. Any decent IDE can do it with a single click for multiple files. I don't want to open the door here for formatting options, which are endless.

KTONQ commented

Understood.
Thanks for the reply.