Skip to content

Commit dc476bc

Browse files
committed
ADFA-2717: Address review of the C/C++ new-file dialog
- Add an Other type so any file (CMakeLists.txt, .hpp) can still be created in a cpp folder. - Size the name counter and validation to the full file name, extension included. - Keep the dialog open until the write succeeds: Create is disabled while the name is invalid, and an existing file shows on the field. - Write all files in one IO job with CREATE_NEW, removing any already written if one fails, and report a single result. - Route cpp folders before java ones, so cpp/javabridge opens the native dialog. - Generate headers with #pragma once instead of a name-derived guard. - Use the existing new-file tooltip tag instead of an undocumented one. - Let the dialog scroll vertically at large font scales. - Cover routing, naming and length rules, and the atomic write in tests.
1 parent cc00fee commit dc476bc

8 files changed

Lines changed: 389 additions & 169 deletions

File tree

‎app/src/main/java/com/itsaky/androidide/actions/FileActionManager.kt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import kotlinx.coroutines.withContext
1111
import org.apache.commons.text.StringEscapeUtils
1212
import org.greenrobot.eventbus.EventBus
1313
import java.io.File
14+
import java.io.IOException
15+
import java.nio.file.Files
16+
import java.nio.file.StandardOpenOption
1417

1518
class FileActionManager {
1619
private val scope = CoroutineScope(Dispatchers.IO)
@@ -51,4 +54,45 @@ class FileActionManager {
5154
}
5255
}
5356
}
57+
58+
fun createNewFiles(
59+
baseDir: File,
60+
files: List<Pair<String, String>>,
61+
onResult: (Result<List<File>>) -> Unit,
62+
) {
63+
scope.launch {
64+
val result =
65+
try {
66+
Result.success(writeNewFiles(baseDir, files))
67+
} catch (e: IOException) {
68+
Result.failure(e)
69+
} catch (e: SecurityException) {
70+
Result.failure(e)
71+
}
72+
result.getOrNull()?.forEach { EventBus.getDefault().post(FileCreationEvent(it)) }
73+
withContext(Dispatchers.Main) { onResult(result) }
74+
}
75+
}
76+
}
77+
78+
internal fun writeNewFiles(
79+
baseDir: File,
80+
files: List<Pair<String, String>>,
81+
): List<File> {
82+
val created = mutableListOf<File>()
83+
try {
84+
for ((path, content) in files) {
85+
val target = File(baseDir, path)
86+
target.parentFile?.let { Files.createDirectories(it.toPath()) }
87+
Files.write(target.toPath(), content.toByteArray(), StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)
88+
created += target
89+
}
90+
} catch (e: IOException) {
91+
created.forEach { it.delete() }
92+
throw e
93+
} catch (e: SecurityException) {
94+
created.forEach { it.delete() }
95+
throw e
96+
}
97+
return created
5498
}

‎app/src/main/java/com/itsaky/androidide/actions/filetree/NewFileAction.kt‎

Lines changed: 91 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
package com.itsaky.androidide.actions.filetree
1919

2020
import android.content.Context
21+
import android.content.DialogInterface
2122
import android.view.LayoutInflater
23+
import androidx.appcompat.app.AlertDialog
2224
import androidx.core.view.isVisible
2325
import com.itsaky.androidide.actions.ActionData
2426
import com.itsaky.androidide.actions.FileActionManager
@@ -51,6 +53,7 @@ import org.koin.core.component.KoinComponent
5153
import org.koin.core.component.get
5254
import org.slf4j.LoggerFactory
5355
import java.io.File
56+
import java.nio.file.FileAlreadyExistsException
5457
import java.util.Objects
5558
import java.util.regex.Pattern
5659

@@ -116,10 +119,6 @@ class NewFileAction(
116119

117120
val projectDir = IProjectManager.getInstance().projectDirPath
118121
Objects.requireNonNull(projectDir)
119-
val isJava =
120-
Pattern.compile(Pattern.quote(projectDir) + JAVA_PATH_REGEX).matcher(file.absolutePath).find()
121-
val isCpp =
122-
Pattern.compile(Pattern.quote(projectDir) + CPP_PATH_REGEX).matcher(file.absolutePath).find()
123122
val isRes =
124123
Pattern.compile(Pattern.quote(projectDir) + RES_PATH_REGEX).matcher(file.absolutePath).find()
125124
val isLayoutRes =
@@ -138,14 +137,18 @@ class NewFileAction(
138137
.matcher(file.absolutePath)
139138
.find()
140139

141-
if (isJava) {
142-
createJavaClass(context, node, file)
143-
return
144-
}
140+
when (sourceDialogFor(projectDir, file.absolutePath)) {
141+
SourceDialog.CPP -> {
142+
createNativeSource(context, node, file)
143+
return
144+
}
145145

146-
if (isCpp) {
147-
createNativeSource(context, node, file)
148-
return
146+
SourceDialog.JAVA -> {
147+
createJavaClass(context, node, file)
148+
return
149+
}
150+
151+
null -> {}
149152
}
150153

151154
if (isLayoutRes && file.name == "layout") {
@@ -329,8 +332,18 @@ class NewFileAction(
329332
directory: File,
330333
) {
331334
val binding = LayoutCreateFileCppBinding.inflate(LayoutInflater.from(context))
332-
binding.languageGroup.addOnButtonCheckedListener { _, _, _ -> refreshNativeDialog(context, binding) }
333-
binding.typeGroup.addOnButtonCheckedListener { _, _, _ -> refreshNativeDialog(context, binding) }
335+
val dialog =
336+
DialogUtils
337+
.newMaterialDialogBuilder(context)
338+
.setView(binding.root)
339+
.setTitle(R.string.new_file)
340+
.setPositiveButton(R.string.text_create, null)
341+
.setNegativeButton(android.R.string.cancel, null)
342+
.setCancelable(false)
343+
.create()
344+
.attachTooltip(TooltipTag.PROJECT_FOLDER_NEWFILE)
345+
binding.languageGroup.addOnButtonCheckedListener { _, _, _ -> refreshNativeDialog(context, binding, dialog) }
346+
binding.typeGroup.addOnButtonCheckedListener { _, _, _ -> refreshNativeDialog(context, binding, dialog) }
334347
binding.name.editText?.addTextChangedListener(
335348
object : SingleTextWatcher() {
336349
override fun onTextChanged(
@@ -339,29 +352,22 @@ class NewFileAction(
339352
before: Int,
340353
count: Int,
341354
) {
342-
refreshNativeDialog(context, binding)
355+
refreshNativeDialog(context, binding, dialog)
343356
}
344357
},
345358
)
346-
refreshNativeDialog(context, binding)
347-
348-
DialogUtils
349-
.newMaterialDialogBuilder(context)
350-
.setView(binding.root)
351-
.setTitle(R.string.new_file)
352-
.setPositiveButton(R.string.text_create) { dialogInterface, _ ->
353-
dialogInterface.dismiss()
354-
doCreateNativeSource(binding, directory, node)
355-
}.setNegativeButton(android.R.string.cancel, null)
356-
.setCancelable(false)
357-
.create()
358-
.attachTooltip(TooltipTag.PROJECT_FOLDER_NEWNATIVE)
359-
.show()
359+
360+
dialog.show()
361+
dialog.getButton(DialogInterface.BUTTON_POSITIVE).setOnClickListener {
362+
doCreateNativeSource(context, binding, dialog, directory, node)
363+
}
364+
refreshNativeDialog(context, binding, dialog)
360365
}
361366

362367
private fun refreshNativeDialog(
363368
context: Context,
364369
binding: LayoutCreateFileCppBinding,
370+
dialog: AlertDialog,
365371
) {
366372
val language = nativeLanguage(binding)
367373
val isCpp = language == NativeSourceBuilder.Language.CPP
@@ -371,37 +377,48 @@ class NewFileAction(
371377
binding.typeClass.isVisible = isCpp
372378

373379
val kind = nativeKind(binding)
374-
binding.name.suffixText = NativeSourceBuilder.extensions(language, kind).joinToString(" + ") { ".$it" }
380+
binding.languageGroup.isEnabled = kind != NativeSourceBuilder.Kind.OTHER
381+
binding.name.suffixText =
382+
NativeSourceBuilder
383+
.extensions(language, kind)
384+
.joinToString(" + ") { ".$it" }
385+
.ifEmpty { null }
386+
binding.name.counterMaxLength = NativeSourceBuilder.maxNameLength(language, kind, MAX_FILE_NAME_LENGTH)
375387

376388
val name = nativeName(binding)
377-
val isInvalid = name.isNotEmpty() && !NativeSourceBuilder.isValidName(name, kind)
378-
binding.name.isErrorEnabled = isInvalid
379-
binding.name.error = if (isInvalid) context.getString(R.string.msg_invalid_name) else null
389+
val isValid = NativeSourceBuilder.isValidName(name, language, kind, MAX_FILE_NAME_LENGTH)
390+
val showError = name.isNotEmpty() && !isValid
391+
binding.name.isErrorEnabled = showError
392+
binding.name.error = if (showError) context.getString(R.string.msg_invalid_name) else null
393+
dialog.getButton(DialogInterface.BUTTON_POSITIVE)?.isEnabled = isValid
380394
}
381395

382396
private fun doCreateNativeSource(
397+
context: Context,
383398
binding: LayoutCreateFileCppBinding,
399+
dialog: AlertDialog,
384400
directory: File,
385401
node: TreeNode?,
386402
) {
387-
val name = nativeName(binding)
388-
val kind = nativeKind(binding)
389-
if (!NativeSourceBuilder.isValidName(name, kind)) {
390-
flashError(R.string.msg_invalid_name)
391-
return
392-
}
393-
394-
val files = NativeSourceBuilder.createFiles(name, nativeLanguage(binding), kind)
395-
if (files.any { it.name.length > MAX_FILE_NAME_LENGTH }) {
396-
flashError(R.string.msg_invalid_name)
397-
return
398-
}
399-
if (files.any { File(directory, it.name).exists() }) {
400-
flashError(R.string.msg_file_exists)
401-
return
403+
val files = NativeSourceBuilder.createFiles(nativeName(binding), nativeLanguage(binding), nativeKind(binding))
404+
val createButton = dialog.getButton(DialogInterface.BUTTON_POSITIVE)
405+
createButton.isEnabled = false
406+
fileActionManager.createNewFiles(directory, files.map { it.name to it.content }) { result ->
407+
result
408+
.onSuccess {
409+
dialog.dismiss()
410+
onFilesCreated(node)
411+
}.onFailure { error ->
412+
createButton.isEnabled = true
413+
if (error is FileAlreadyExistsException) {
414+
binding.name.isErrorEnabled = true
415+
binding.name.error = context.getString(R.string.msg_file_exists)
416+
} else {
417+
log.error("Failed to create native source files", error)
418+
flashError(error.message)
419+
}
420+
}
402421
}
403-
404-
files.forEach { createFile(node, directory, it.name, it.content) }
405422
}
406423

407424
private fun nativeName(binding: LayoutCreateFileCppBinding): String =
@@ -422,6 +439,7 @@ class NewFileAction(
422439
binding.typeSource.id -> NativeSourceBuilder.Kind.SOURCE
423440
binding.typeHeader.id -> NativeSourceBuilder.Kind.HEADER
424441
binding.typeClass.id -> NativeSourceBuilder.Kind.CLASS
442+
binding.typeOther.id -> NativeSourceBuilder.Kind.OTHER
425443
else -> error("Unexpected type button: $id")
426444
}
427445

@@ -610,10 +628,14 @@ class NewFileAction(
610628
message: String,
611629
createdFile: File?,
612630
) {
631+
onFilesCreated(currentNode)
632+
}
633+
634+
private fun onFilesCreated(node: TreeNode?) {
613635
flashSuccess(R.string.msg_file_created)
614-
if (currentNode != null) {
615-
requestCollapseNode(currentNode!!, false)
616-
requestExpandNode(currentNode!!)
636+
if (node != null) {
637+
requestCollapseNode(node, false)
638+
requestExpandNode(node)
617639
} else {
618640
requestFileListing()
619641
}
@@ -623,3 +645,20 @@ class NewFileAction(
623645
flashError(errorMessage)
624646
}
625647
}
648+
649+
internal enum class SourceDialog {
650+
CPP,
651+
JAVA,
652+
}
653+
654+
internal fun sourceDialogFor(
655+
projectDir: String,
656+
path: String,
657+
): SourceDialog? {
658+
fun matches(regex: String) = Pattern.compile(Pattern.quote(projectDir) + regex).matcher(path).find()
659+
return when {
660+
matches(NewFileAction.CPP_PATH_REGEX) -> SourceDialog.CPP
661+
matches(NewFileAction.JAVA_PATH_REGEX) -> SourceDialog.JAVA
662+
else -> null
663+
}
664+
}

‎app/src/main/java/com/itsaky/androidide/utils/NativeSourceBuilder.kt‎

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package com.itsaky.androidide.utils
22

3-
import java.util.Locale
4-
53
object NativeSourceBuilder {
64
enum class Language(
75
val sourceExtension: String,
@@ -14,6 +12,7 @@ object NativeSourceBuilder {
1412
SOURCE,
1513
HEADER,
1614
CLASS,
15+
OTHER,
1716
}
1817

1918
data class NativeFile(
@@ -131,61 +130,72 @@ object NativeSourceBuilder {
131130
Kind.SOURCE -> listOf(language.sourceExtension)
132131
Kind.HEADER -> listOf(HEADER_EXTENSION)
133132
Kind.CLASS -> listOf(HEADER_EXTENSION, requireCpp(language).sourceExtension)
133+
Kind.OTHER -> emptyList()
134134
}
135135

136+
fun maxNameLength(
137+
language: Language,
138+
kind: Kind,
139+
maxFileNameLength: Int,
140+
): Int = maxFileNameLength - (extensions(language, kind).maxOfOrNull { it.length + 1 } ?: 0)
141+
136142
fun isValidName(
137143
name: String,
144+
language: Language,
138145
kind: Kind,
139-
): Boolean =
140-
when (kind) {
141-
Kind.CLASS -> IDENTIFIER.matches(name) && name !in CPP_KEYWORDS
142-
Kind.SOURCE, Kind.HEADER -> FILE_NAME.matches(name)
143-
}
146+
maxFileNameLength: Int,
147+
): Boolean = followsNamingRule(name, kind) && name.length <= maxNameLength(language, kind, maxFileNameLength)
144148

145149
fun createFiles(
146150
name: String,
147151
language: Language,
148152
kind: Kind,
149153
): List<NativeFile> {
150-
require(isValidName(name, kind)) { "Invalid $kind name: '$name'" }
154+
require(followsNamingRule(name, kind)) { "Invalid $kind name: '$name'" }
151155
val headerName = "$name.$HEADER_EXTENSION"
152156
return when (kind) {
153157
Kind.SOURCE -> {
154158
listOf(NativeFile("$name.${language.sourceExtension}", ""))
155159
}
156160

157161
Kind.HEADER -> {
158-
listOf(NativeFile(headerName, header(name, body = null)))
162+
listOf(NativeFile(headerName, header(body = null)))
159163
}
160164

161165
Kind.CLASS -> {
162166
listOf(
163-
NativeFile(headerName, header(name, body = "class $name {\n};\n")),
167+
NativeFile(headerName, header(body = "class $name {\n};\n")),
164168
NativeFile("$name.${requireCpp(language).sourceExtension}", "#include \"$headerName\"\n"),
165169
)
166170
}
171+
172+
Kind.OTHER -> {
173+
listOf(NativeFile(name, ""))
174+
}
167175
}
168176
}
169177

178+
private fun followsNamingRule(
179+
name: String,
180+
kind: Kind,
181+
): Boolean =
182+
when (kind) {
183+
Kind.CLASS -> IDENTIFIER.matches(name) && name !in CPP_KEYWORDS
184+
Kind.SOURCE, Kind.HEADER -> FILE_NAME.matches(name)
185+
Kind.OTHER -> name.split('/').all { it.isNotBlank() && it != "." && it != ".." }
186+
}
187+
170188
private fun requireCpp(language: Language): Language {
171189
require(language == Language.CPP) { "A class needs C++, not $language" }
172190
return language
173191
}
174192

175-
private fun header(
176-
name: String,
177-
body: String?,
178-
): String {
179-
val guard = "${name.uppercase(Locale.ROOT).replace('-', '_')}_H"
180-
return buildString {
181-
appendLine("#ifndef $guard")
182-
appendLine("#define $guard")
183-
appendLine()
193+
private fun header(body: String?): String =
194+
buildString {
195+
appendLine("#pragma once")
184196
if (body != null) {
185-
append(body)
186197
appendLine()
198+
append(body)
187199
}
188-
appendLine("#endif")
189200
}
190-
}
191201
}

0 commit comments

Comments
 (0)