Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make pathToParser and pathToPsiRoot optional #178

Open
JojOatXGME opened this issue Mar 30, 2024 · 0 comments · May be fixed by #179
Open

Make pathToParser and pathToPsiRoot optional #178

JojOatXGME opened this issue Mar 30, 2024 · 0 comments · May be fixed by #179

Comments

@JojOatXGME
Copy link
Contributor

JojOatXGME commented Mar 30, 2024

Is your feature request related to a problem? Please describe.

When applying this plugin, you have to configure the tasks. The GenerateParserTask has the following two mandatory properties.

  • pathToParser
  • pathToPsiRoot

I was wondering why these properties are needed, and it seems that they are used only if the optional property purgeOldFiles is set to true. As the value of these properties seems to be unused in all other scenarios, I would like to not have to specify them. Also note that these properties duplicate information which is already specified in the *.bnf file. Therefore, having to specify these properties is not only unnecessary, but also opens up the possibility for inconsistent configurations.

Describe the solution you'd like

  • Mark pathToParser as optional
  • Mark pathToPsiRoot as optional

Describe alternatives you've considered

A maybe better but more elaborate change would be to remove/deprecate GenerateParserTask.purgeOldFiles, GenerateLexerTask.purgeOldFiles and the two options mentioned above. We should always delete the whole directory specified by GenerateParserTask.targetRootOutputDir and GenerateLexerTask.targetOutputDir. The selective deletion implemented right now is quite prone to misconfiguration anyway. It is also only relevant when sharing output directories between multiple tasks, which is not recommended because it can cause issues with caching.

To make the transition easier for people who are currently using a shared output directory for generateLexer and generateParser, we can provide proper defaults for GenerateParserTask.targetRootOutputDir and GenerateLexerTask.targetOutputDir. After this has been done, the only options which still need to be configured by the user are GenerateLexerTask.sourceFile and GenerateParserTask.sourceFile. To make the configuration more convenient, we can provide a property for these options in GrammarKitPluginExtension. With all these changes, the user only needs to make the following configuration for a working build:

grammarKit {
    lexerSource = layout.projectDirectory.file("src/main/lang/Nix.flex")
    parserSource = layout.projectDirectory.file("src/main/lang/Nix.bnf")
}

Right now, the minimal configuration looks like the following, and it is not even working properly when a public parser rule is removed, as the generated file will not be deleted:

val lexerSource = layout.buildDirectory.dir("generated/sources/lexer/java/main")

sourceSets {
    main {
        java {
            srcDir(lexerSource)
            srcDir(tasks.generateParser)
        }
    }
}

tasks {
    generateLexer {
        sourceFile = layout.projectDirectory.file("src/main/lang/Nix.flex")
        targetOutputDir = lexerSource.map { it.dir("org/nixos/idea/lang") }
    }

    generateParser {
        sourceFile = layout.projectDirectory.file("src/main/lang/Nix.bnf")
        targetRootOutputDir = layout.buildDirectory.dir("generated/sources/parser/java/main")
        pathToParser = "org/nixos/idea/lang/NixParser.java"
        pathToPsiRoot = "org/nixos/idea/psi"
    }

    compileJava {
        dependsOn(generateLexer)
    }
}

PS: I would be willing to prepare a PR with either of the two solutions.

@JojOatXGME JojOatXGME linked a pull request Apr 1, 2024 that will close this issue
10 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant