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

initial formatter schema #12

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
69 changes: 56 additions & 13 deletions src/language-server/universal-data-definition-language-formatter.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { AbstractFormatter, AstNode, Formatting, Module, PartialLangiumServices } from 'langium';
import { AbstractFormatter, AstNode, Formatting, Module, PartialLangiumServices, } from 'langium';
import * as ast from './generated/ast';
import { UniversalDataDefinitionLanguageServices } from './universal-data-definition-language-module';

export class UniversalDataDefinitionLanguageFormatter extends AbstractFormatter {

protected formatContainer(node: AstNode): void {
const formatter = this.getNodeFormatter(node);
const open = formatter.keyword('{');
Expand All @@ -22,12 +21,22 @@ export class UniversalDataDefinitionLanguageFormatter extends AbstractFormatter
close.surround(Formatting.noSpace()).prepend(Formatting.newLine({allowMore: true})).append(Formatting.newLine({allowMore: true}));
}

protected format(node: AstNode): void {
protected formatNode(node: AstNode): void {
const formatter = this.getNodeFormatter(node);
formatter.property('name').prepend(Formatting.newLine()).surround(Formatting.oneSpace({allowMore: true}))
}

protected format(node: AstNode): void {
// This method is called for every AstNode in a document
if (ast.isDataModel(node)) {
this.formatDataModel(node);

}
else if(ast.isConceptualAssociation(node)){
this.formatConceptualAssociation(node)
}
else if(ast.isConceptualEntity(node)){
this.formatConceptualEntity(node)
}
}


Expand All @@ -50,10 +59,10 @@ export class UniversalDataDefinitionLanguageFormatter extends AbstractFormatter

protected formatConceptualDataModel(cdm: ast.ConceptualDataModel): void {
this.formatContainer(cdm);
cdm.cdm.forEach(dm => {
cdm.cdm.forEach(dm => {
this.formatConceptualDataModel(dm);
})
cdm.element.forEach( elem => {
cdm.element.forEach( elem => {
this.formatConceptualElement(elem);
})
}
Expand All @@ -64,7 +73,7 @@ export class UniversalDataDefinitionLanguageFormatter extends AbstractFormatter
this.formatLogicalDataModel(dm)
})
ldm.element.forEach( elem => {
this.formatElement(elem)
this.formatLogicalElement(elem)
})
}

Expand All @@ -74,17 +83,52 @@ export class UniversalDataDefinitionLanguageFormatter extends AbstractFormatter
this.formatPlatfornDataModel(dm)
})
pdm.element.forEach(elem => {
this.formatElement(elem)
this.formatPlatformElement(elem)
})
}

protected formatConceptualElement(elem: ast.ConceptualElement): void {
/**Formatting conceptuals */
protected formatConceptualElement(elem: ast.ConceptualElement): void {
const formatter = this.getNodeFormatter(elem);
formatter.property('name').prepend(Formatting.newLine()).surround(Formatting.oneSpace({allowMore: true}));
formatter.property('name').prepend(Formatting.newLine({allowMore: true})).surround(Formatting.oneSpace({allowMore: true}));
this.formatObj(elem);
}

protected formatElement(elem: ast.LogicalElement | ast.PlatformElement): void {
protected formatConceptualEntity(entity: ast.ConceptualEntity): void {
this.formatContainer(entity)
entity.composition.forEach(comp => {
this.formatConceptualComposition(comp)
})
}

protected formatConceptualAssociation(cassoc: ast.ConceptualAssociation): void {
this.formatContainer(cassoc)

cassoc.composition.forEach(comp => {
this.formatConceptualComposition(comp)
})
}

protected formatConceptualComposition(comp: ast.ConceptualComposition ): void {
this.formatContainer(comp)
}

protected formatConceptualParticipant(participant: ast.ConceptualParticipant): void {
this.formatContainer(participant)

if(participant?.type){
this.formatConceptualEntity(participant.type.ref!)
}
}

/**Formatting logicals */
protected formatLogicalElement(elem: ast.LogicalElement): void {
const formatter = this.getNodeFormatter(elem);
formatter.property('name').prepend(Formatting.newLine()).surround(Formatting.oneSpace({allowMore: true}));
this.formatObj(elem);
}

protected formatPlatformElement(elem: ast.PlatformElement): void {
const formatter = this.getNodeFormatter(elem);
formatter.property('name').prepend(Formatting.newLine()).surround(Formatting.oneSpace({allowMore: true}));
this.formatObj(elem);
Expand All @@ -97,5 +141,4 @@ export const UniversalDataDefinitionLanguageModule: Module<UniversalDataDefiniti
lsp: {
Formatter: () => new UniversalDataDefinitionLanguageFormatter()
}
};

};
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,125 @@ import { createUniversalDataDefinitionLanguageServices } from "../../src/languag
import { EmptyFileSystem } from "langium";
import { expectFormatting } from "langium/test";

const universalDataDefinitionLanguageService =createUniversalDataDefinitionLanguageServices({...EmptyFileSystem,}).UniversalDataDefinitionLanguage;
const universalDataDefinitionLanguageFormatting = expectFormatting(universalDataDefinitionLanguageService
);
const universalDataDefinitionLanguageService = createUniversalDataDefinitionLanguageServices({...EmptyFileSystem,}).UniversalDataDefinitionLanguage;
const universalDataDefinitionLanguageFormatting = expectFormatting(universalDataDefinitionLanguageService);

describe("Universal Data Definition Language Formatter", () => {
it("should indent correctly", async () => {
it("should format cdm array", async () => {
await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{}}',
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{cdm anothercdm "A cdm with another data" {observable Information "Something a party can learn";}}}',
after: `dm PPT "Base data structures to support People, Places and Things"
{

cdm Conceptual "Need to start at Conceptual Level"
{

cdm anothercdm "A cdm with another data"
{

observable Information "Something a party can learn";
}

}

}`
}`,
});
});

it("should format cdm with conceptual association", async () => {
await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{cassoc Training "Information delivered over time from one party to another " {};}}',
after: `dm PPT "Base data structures to support People, Places and Things"
{

cdm Conceptual "Need to start at Conceptual Level"
{

cassoc Training "Information delivered over time from one party to another "
{

};

}

}`,
});
});

it("should format cdm with conceptual entity", async () => {
await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{centity AddressableEntity "Any entity that is addressable in some way" {};}}',
Copy link
Contributor

Choose a reason for hiding this comment

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

In your current tests, all the 'before' text is on a single line. You should also have the same tests where the 'before' text has each token on a separate line (you can do this with a find/replace that replaces blanks the before text with CRs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted

after: `dm PPT "Base data structures to support People, Places and Things"
{

cdm Conceptual "Need to start at Conceptual Level"
{

centity AddressableEntity "Any entity that is addressable in some way"
{

};

}

}`,
});
});

it('should format cdm with conceptual basis',async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

While you can have tests for each individual type, you may find that a test that combines multiple types is sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a previous commit, I merged conceptual entities and conceptual associations into one test case. During this process, only formatter for conceptual entities was checked. I then attempted to verify whether I would encounter similar behavior for conceptual basis and conceptual domain as well.

await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{basis uddlBasis "Formating conceptual basis entity"; }}',
after: `dm PPT "Base data structures to support People, Places and Things"
{

cdm Conceptual "Need to start at Conceptual Level"
{

basis uddlBasis "Formating conceptual basis entity";
}

}`,
})
})

it('should format cdm with conceptual domain',async () => {
await universalDataDefinitionLanguageFormatting({
before:
'dm PPT "Base data structures to support People, Places and Things"{cdm Conceptual "Need to start at Conceptual Level"{domain NaturalPerson "Base definition of a natural person"; }}',
after: `dm PPT "Base data structures to support People, Places and Things"
{

cdm Conceptual "Need to start at Conceptual Level"
{

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really want all these blank lines after formatting? You've got a blank line after every '{' and '}'. That's going to make some files a lot longer - which will force users to scroll. When people have to scroll, they forget what they can't see - which makes it harder to understand complex content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test would not pass without the blank lines in the after block

Copy link
Contributor

Choose a reason for hiding this comment

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

When a test does not pass, you have a choice: change the test (what you did) or change the code being tested. The point I'm making above is that the formatter currently inserts those blank lines - which is why your test needs the blank lines to pass. My question is: do you want the formatter to do that? The goal of formatting is to put the text into a format that is easy to read and understand. Every decision you make about how the formatter works should be made with that in mind.

domain NaturalPerson "Base definition of a natural person";
}

}`,
})
})

it("should format ldm array",async () => {
await universalDataDefinitionLanguageFormatting({
before: 'dm Book "Base definition for book requirement" {ldm library "contains available books" {ldm BookShelf "Books related by category"{}}}',
after: `dm Book "Base definition for book requirement"
{

ldm library "contains available books"
{

ldm BookShelf "Books related by category"
{

}

}

}`
})
})
});