Skip to content

Commit 656d91d

Browse files
author
Ahmed R. Mohamed
committed
ord: fix tcl completion issue #7816
Use ::sta::cmd_args to find the command args and combine them with file completions and ScriptCompleter results. ScriptCompleter runs the command with args "----" to find its completions, which can have side effects! Maybe we should remove it entirely in a follow up PR. Fixes #7816 Signed-off-by: Ahmed R. Mohamed <[email protected]>
1 parent 7de25a9 commit 656d91d

File tree

5 files changed

+131
-8
lines changed

5 files changed

+131
-8
lines changed

src/Main.cc

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -313,14 +313,9 @@ int main(int argc, char* argv[])
313313
#ifdef ENABLE_READLINE
314314
static int tclReadlineInit(Tcl_Interp* interp)
315315
{
316-
std::array<const char*, 7> readline_cmds = {
317-
"history event",
318-
"eval $auto_index(::tclreadline::ScriptCompleter)",
319-
"::tclreadline::readline builtincompleter true",
320-
"::tclreadline::readline customcompleter ::tclreadline::ScriptCompleter",
321-
"proc ::tclreadline::prompt1 {} { return \"openroad> \" }",
322-
"proc ::tclreadline::prompt2 {} { return \"...> \" }",
323-
"::tclreadline::Loop"};
316+
std::array<const char*, 2> readline_cmds = {
317+
"ord::setup_tclreadline",
318+
"::tclreadline::Loop"};
324319

325320
for (auto cmd : readline_cmds) {
326321
if (TCL_ERROR == Tcl_Eval(interp, cmd)) {

src/OpenRoad.tcl

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,5 +587,51 @@ proc parse_list_args { cmd arg_var list_var lists_args } {
587587
}
588588
}
589589

590+
proc cmd_args_completer {part start end line} {
591+
if { $start == 0 || [catch {set cmd [::tclreadline::Lindex $line 0]} err] != 0 } {
592+
utl::redirectStringBegin
593+
set err [catch {::tclreadline::ScriptCompleter $part $start $end $line} ret]
594+
utl::redirectStringEnd
595+
if { $err == 0} {
596+
return $ret
597+
} else {
598+
return ""
599+
}
600+
}
601+
if { [info exists ::sta::cmd_args($cmd)] } {
602+
set metadata $::sta::cmd_args($cmd)
603+
}
604+
set unused_flags {}
605+
if { [info exists metadata] } {
606+
set all_flags [regexp -all -inline -- {[-+][a-zA-Z0-9_]+} $metadata]
607+
set unused_flags [::tclreadline::RemoveUsedOptions $line [lsort -unique $all_flags]]
608+
}
609+
set raw_files [glob -nocomplain -types {f d} -- "${part}*"]
610+
set files {}
611+
foreach f $raw_files {
612+
if {[file isdirectory $f]} {
613+
lappend files "${f}/"
614+
} else {
615+
lappend files $f
616+
}
617+
}
618+
set matches [lsort -unique [concat $unused_flags $files]]
619+
set matches [::tclreadline::CompleteFromList $part $matches]
620+
if { [llength $matches] == 1 && [string index [lindex $matches 0] end] == "/" } {
621+
return [list [lindex $matches 0] [list {}]]
622+
}
623+
return $matches
624+
}
625+
626+
proc setup_tclreadline { } {
627+
history event
628+
eval $::auto_index(::tclreadline::ScriptCompleter)
629+
::tclreadline::readline builtincompleter false
630+
catch { ::tclreadline::ScriptCompleter "" 0 0 "" }
631+
::tclreadline::readline customcompleter ::ord::cmd_args_completer
632+
proc ::tclreadline::prompt1 {} { return "openroad> " }
633+
proc ::tclreadline::prompt2 {} { return "...> " }
634+
}
635+
590636
# namespace ord
591637
}

test/issue_7816.ok

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Test 1: Command name completion 'read_db'
2+
PASS: Test 1 returned empty (as expected for mock failure)
3+
4+
Test 2: Argument completion 'read_db '
5+
PASS: Test 2 returned list of length > 0
6+
7+
Test 3: write_lef completion
8+
PASS: Test 3 returned list of length > 0

test/issue_7816.tcl

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# Mock tclreadline
2+
namespace eval ::tclreadline {
3+
proc Lindex {line index} {
4+
return [lindex $line $index]
5+
}
6+
7+
proc ScriptCompleter {part start end line} {
8+
# Simulate noisy output from ScriptCompleter to ensure it's suppressed
9+
puts "ScriptCompleter: [ERROR] Should be suppressed"
10+
error "ScriptCompleter Error"
11+
}
12+
13+
proc RemoveUsedOptions {line flags} {
14+
# Default behavior: return flags as is
15+
return $flags
16+
}
17+
18+
proc CompleteFromList {part matches} {
19+
set res {}
20+
foreach m $matches {
21+
if {[string match "${part}*" $m]} {
22+
lappend res $m
23+
}
24+
}
25+
return $res
26+
}
27+
}
28+
29+
if {[info procs ::ord::cmd_args_completer] eq ""} {
30+
puts "Error: ::ord::cmd_args_completer not found."
31+
exit 1
32+
}
33+
34+
puts "Test 1: Command name completion 'read_db'"
35+
# start=0 triggers ScriptCompleter path.
36+
# We expect NO error to propagate (caught inside) and clean stdout (redirected).
37+
# Result should be "" (from catch block returning "")
38+
set result ""
39+
if { [catch {set result [::ord::cmd_args_completer "read_db" 0 7 "read_db"]} err] } {
40+
puts "FAIL: Test 1 threw error: $err"
41+
} else {
42+
if {$result eq ""} {
43+
puts "PASS: Test 1 returned empty (as expected for mock failure)"
44+
} else {
45+
puts "PASS: Test 1 returned: $result"
46+
}
47+
}
48+
49+
puts "\nTest 2: Argument completion 'read_db '"
50+
# start=8 triggers RemoveUsedOptions path.
51+
# We expect it to find files in current directory (or our mock).
52+
# Since we didn't mock glob, it will use real glob.
53+
# 'read_db' takes a file. cmd_args_completer logic does `glob`.
54+
set result ""
55+
if { [catch {set result [::ord::cmd_args_completer "" 8 8 "read_db "]} err] } {
56+
puts "FAIL: Test 2 threw error: $err"
57+
} else {
58+
# It should return a list of files/dirs
59+
if {[llength $result] > 0} {
60+
puts "PASS: Test 2 returned list of length > 0"
61+
}
62+
}
63+
64+
puts "\nTest 3: write_lef completion"
65+
# write_lef also takes filename.
66+
if { [catch {set result [::ord::cmd_args_completer "" 10 10 "write_lef "]} err] } {
67+
puts "FAIL: Test 3 threw error: $err"
68+
} else {
69+
if {[llength $result] > 0} {
70+
puts "PASS: Test 3 returned list of length > 0"
71+
}
72+
}

test/regression_tests.tcl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,5 @@ record_flow_tests {
2424
record_test open_db $test_dir "compare_logfile" "-db gcd_sky130hd.odb"
2525
# For invalid DB, allow non-zero exit but require log to match ok
2626
record_test open_db_invalid $test_dir "compare_logfile_allow_error" "-db nonexistent_file.odb"
27+
28+
record_test issue_7816 $test_dir "compare_logfile"

0 commit comments

Comments
 (0)