Skip to content

feat: add interpolation for config options #238

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
+406 −62
Closed

feat: add interpolation for config options #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@
"istanbul": "^0.4.5",
"lint-staged": "^7.3.0",
"mocha": "^2.3.3",
"prettier": "^2.3.1",
"remap-istanbul": "^0.8.4",
"tslint": "^4.0.2",
"typescript": "^3.5.1",
Expand Down
18 changes: 11 additions & 7 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,27 @@ import { FORTRAN_FREE_FORM_ID, EXTENSION_ID } from './lib/helper'
import { FortranLangServer, checkForLangServer } from './lang-server'
import { LoggingService } from './services/logging-service'
import * as pkg from '../package.json'
import { Config } from './services/config'

export function activate(context: vscode.ExtensionContext) {
export async function activate(context: vscode.ExtensionContext) {
const loggingService = new LoggingService()
const extensionConfig = vscode.workspace.getConfiguration(EXTENSION_ID)

const extensionConfig = new Config(
vscode.workspace.getConfiguration(EXTENSION_ID),
loggingService
)
loggingService.logInfo(`Extension Name: ${pkg.displayName}`)
loggingService.logInfo(`Extension Version: ${pkg.version}`)

if (extensionConfig.get('linterEnabled', true)) {
let linter = new FortranLintingProvider(loggingService)
if (await extensionConfig.get('linterEnabled', true)) {
let linter = new FortranLintingProvider(loggingService, extensionConfig)
linter.activate(context.subscriptions)
vscode.languages.registerCodeActionsProvider(FORTRAN_FREE_FORM_ID, linter)
} else {
loggingService.logInfo('Linter is not enabled')
}

if (extensionConfig.get('provideCompletion', true)) {
if (await extensionConfig.get('provideCompletion', true)) {
let completionProvider = new FortranCompletionProvider(loggingService)
vscode.languages.registerCompletionItemProvider(
FORTRAN_FREE_FORM_ID,
Expand All @@ -36,14 +40,14 @@ export function activate(context: vscode.ExtensionContext) {
loggingService.logInfo('Completion Provider is not enabled')
}

if (extensionConfig.get('provideHover', true)) {
if (await extensionConfig.get('provideHover', true)) {
let hoverProvider = new FortranHoverProvider(loggingService)
vscode.languages.registerHoverProvider(FORTRAN_FREE_FORM_ID, hoverProvider)
} else {
loggingService.logInfo('Hover Provider is not enabled')
}

if (extensionConfig.get('provideSymbols', true)) {
if (await extensionConfig.get('provideSymbols', true)) {
let symbolProvider = new FortranDocumentSymbolProvider()
vscode.languages.registerDocumentSymbolProvider(
FORTRAN_FREE_FORM_ID,
Expand Down
142 changes: 87 additions & 55 deletions src/features/linter-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,23 @@ import { getIncludeParams, LANGUAGE_ID } from '../lib/helper'

import * as vscode from 'vscode'
import { LoggingService } from '../services/logging-service'
import { Config } from '../services/config'
import { EnvironmentVariables } from '../services/utils'

const ERROR_REGEX: RegExp =
/^([a-zA-Z]:\\)*([^:]*):([0-9]+):([0-9]+):\s+(.*)\s+.*?\s+(Error|Warning|Fatal Error):\s(.*)$/gm

const knownModNames = ['mpi']
Copy link
Member

Choose a reason for hiding this comment

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

The errors for the missing mpi.mod are valid and should not be ignored. As in, gfortran does not automatically include the path where mpi.mod is located. Adding path interpolation + glob support should be able to fix that.

People often forget that they need to configure their extension include paths and compiler arguments, similarly to how they wish to compile their code, otherwise linting results will not be very useful and they will be littered with errors.


export default class FortranLintingProvider {
constructor(private loggingService: LoggingService) {}
constructor(
private loggingService: LoggingService,
private _config: Config
) {}

private diagnosticCollection: vscode.DiagnosticCollection

private doModernFortranLint(textDocument: vscode.TextDocument) {
const errorRegex: RegExp =
/^([a-zA-Z]:\\)*([^:]*):([0-9]+):([0-9]+):\s+(.*)\s+.*?\s+(Error|Warning|Fatal Error):\s(.*)$/gm

private async doModernFortranLint(textDocument: vscode.TextDocument) {
if (
textDocument.languageId !== LANGUAGE_ID ||
textDocument.uri.scheme !== 'file'
Expand All @@ -24,9 +31,9 @@ export default class FortranLintingProvider {
}

let decoded = ''
let diagnostics: vscode.Diagnostic[] = []
let command = this.getGfortranPath()
let argList = this.constructArgumentList(textDocument)

let command = await this.getGfortranPath()
let argList = await this.constructArgumentList(textDocument)

let filePath = path.parse(textDocument.fileName).dir

Expand All @@ -37,70 +44,82 @@ export default class FortranLintingProvider {
*
* see also: https://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html
*/
const env = process.env
env.LC_ALL = 'C'
const env: EnvironmentVariables = { ...process.env, LC_ALL: 'C' }
if (process.platform === 'win32') {
// Windows needs to know the path of other tools
if (!env.Path.includes(path.dirname(command))) {
env.Path = `${path.dirname(command)}${path.delimiter}${env.Path}`
}
}
let childProcess = cp.spawn(command, argList, { cwd: filePath, env: env })
this.loggingService.logInfo(
`executing linter command ${command} ${argList.join(' ')}`
)
let gfortran = cp.spawn(command, argList, { cwd: filePath, env })

if (childProcess.pid) {
childProcess.stdout.on('data', (data: Buffer) => {
if (gfortran && gfortran.pid) {
gfortran!.stdout!.on('data', (data: Buffer) => {
decoded += data
})
childProcess.stderr.on('data', (data) => {
gfortran!.stderr!.on('data', (data) => {
decoded += data
})
childProcess.stderr.on('end', () => {
let matchesArray: string[]
while ((matchesArray = errorRegex.exec(decoded)) !== null) {
let elements: string[] = matchesArray.slice(1) // get captured expressions
let startLine = parseInt(elements[2])
let startColumn = parseInt(elements[3])
let type = elements[5] // error or warning
let severity =
type.toLowerCase() === 'warning'
? vscode.DiagnosticSeverity.Warning
: vscode.DiagnosticSeverity.Error
let message = elements[6]
let range = new vscode.Range(
new vscode.Position(startLine - 1, startColumn),
new vscode.Position(startLine - 1, startColumn)
)
let diagnostic = new vscode.Diagnostic(range, message, severity)
diagnostics.push(diagnostic)
}

this.diagnosticCollection.set(textDocument.uri, diagnostics)
gfortran!.stderr.on('end', () => {
this.reportErrors(decoded, textDocument)
})
childProcess.stdout.on('close', (code) => {
gfortran.stdout.on('close', (code) => {
console.log(`child process exited with code ${code}`)
})
} else {
childProcess.on('error', (err: any) => {
gfortran.on('error', (err: any) => {
if (err.code === 'ENOENT') {
vscode.window.showErrorMessage(
"gfortran can't found on path, update your settings with a proper path or disable the linter."
"gfortran executable can't be found at the provided path, update your settings with a proper path or disable the linter."
)
}
})
}
}

private constructArgumentList(textDocument: vscode.TextDocument): string[] {
let options = vscode.workspace.rootPath
? { cwd: vscode.workspace.rootPath }
: undefined
reportErrors(errors: string, textDocument: vscode.TextDocument) {
let diagnostics: vscode.Diagnostic[] = []
let matchesArray: string[]
while ((matchesArray = ERROR_REGEX.exec(errors)) !== null) {
let elements: string[] = matchesArray.slice(1) // get captured expressions
let startLine = parseInt(elements[2])
let startColumn = parseInt(elements[3])
let type = elements[5] // error or warning
let severity =
type.toLowerCase() === 'warning'
? vscode.DiagnosticSeverity.Warning
: vscode.DiagnosticSeverity.Error
let message = elements[6]
const [isModError, modName] = isModuleMissingErrorMessage(message)
// skip error from known mod names
if (isModError && knownModNames.includes(modName)) {
continue
}
let range = new vscode.Range(
new vscode.Position(startLine - 1, startColumn),
new vscode.Position(startLine - 1, startColumn)
)

let diagnostic = new vscode.Diagnostic(range, message, severity)
diagnostics.push(diagnostic)
}

this.diagnosticCollection.set(textDocument.uri, diagnostics)
}

private async constructArgumentList(
textDocument: vscode.TextDocument
): Promise<string[]> {
let args = [
'-fsyntax-only',
'-cpp',
'-fdiagnostics-show-option',
...this.getLinterExtraArgs(),
...(await this.getLinterExtraArgs()),
]
let includePaths = this.getIncludePaths()
let includePaths = await this.getIncludePaths()

let extensionIndex = textDocument.fileName.lastIndexOf('.')
let fileNameWithoutExtension = textDocument.fileName.substring(
Expand Down Expand Up @@ -164,21 +183,34 @@ export default class FortranLintingProvider {
this.command.dispose()
}

private getIncludePaths(): string[] {
let config = vscode.workspace.getConfiguration('fortran')
let includePaths: string[] = config.get('includePaths', [])

private async getIncludePaths(): Promise<string[]> {
let includePaths: string[] = await this._config.get('includePaths', [])
this.loggingService.logInfo(`using include paths "${includePaths}"`)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe output dirs one per line? using include paths:\n ${includePaths.join('\r\n')}.

It would also be a good idea to validate that the include paths exist, although it might become computationally expensive if checked every time we open a file.

return includePaths
}
private getGfortranPath(): string {
let config = vscode.workspace.getConfiguration('fortran')
const gfortranPath = config.get('gfortranExecutable', 'gfortran')
this.loggingService.logInfo(`using gfortran executable: ${gfortranPath}`)

private async getGfortranPath(): Promise<string> {
const gfortranPath = await this._config.get(
'gfortranExecutable',
'gfortran'
)
this.loggingService.logInfo(`using gfortran executable: "${gfortranPath}"`)
return gfortranPath
}

private getLinterExtraArgs(): string[] {
let config = vscode.workspace.getConfiguration('fortran')
return config.get('linterExtraArgs', ['-Wall'])
private getLinterExtraArgs(): Promise<string[]> {
return this._config.get('linterExtraArgs', ['-Wall'])
}
}

function isModuleMissingErrorMessage(
message: string
): [boolean, string | null] {
const result = /^Cannot open module file '(\w+).mod' for reading/.exec(
message
)
if (result) {
return [true, result[1]]
}
return [false, null]
}
Loading