Solved: bookmark from text file via comboboxes

[white_flag]

Good morning
I am stuck in this code and I need your help.
I try to insert some text via comboxes from an text file

Private Sub fill()
nFile = FreeFile()
Set fs = CreateObject("Scripting.FileSystemObject")
For j = 1 To 3
If Controls("OptionButton" & j) = True Then
Set f = fs.GetFile(ActiveDocument.Path & "\bkmk." & j)
Open ActiveDocument.Path & "\bkmk." & j For Input As #nFile
Do While Not EOF(nFile)
Input #nFile, sTemp
For i = 1 To 2
If Len(sTemp) > 0 Then
nEqualsSignPos = InStr(sTemp, "=")
nEqualsSignPos = nEqualsSignPos + 1
If InStr(sTemp, Controls("ComboBox" & i).Value) Then
Controls("Frame" & i).Caption = Mid(sTemp, nEqualsSignPos)
Call FillBM("book" & i, Controls("Frame" & i).Caption)
End If
End If
Next
Loop
Close nFile
End If
Next
End Sub

1. I like to change this line to be without Controls("Frame" & i).Caption

Controls("Frame" & i).Caption = Mid(sTemp, nEqualsSignPos)
Call FillBM("book" & i, Controls("Frame" & i).Caption)
2. if the combobox is empty on bookmark is insert the last value from text. it is possible to insert nothing if the selection of combo is null

thank you

attachment is more clear for what I like to do

[white_flag]

Private Sub fill()
nFile = FreeFile()
Set fs = CreateObject("Scripting.FileSystemObject")
For j = 1 To 3
If Controls("OptionButton" & j) = True Then
Set f = fs.GetFile(ActiveDocument.Path & "\bkmk." & j)
Open ActiveDocument.Path & "\bkmk." & j For Input As #nFile
Do While Not EOF(nFile)
Input #nFile, sTemp
For i = 1 To 2
If Len(sTemp) > 0 Then
nEqualsSignPos = InStr(sTemp, "=")
nEqualsSignPos = nEqualsSignPos + 1
If InStr(sTemp, Controls("ComboBox" & i).Value) Then
Call FillBM("book" & i, Mid(sTemp, nEqualsSignPos))
End If
End If
Next
Loop
Close nFile
End If
Next
End Sub


I can not figure out how to insert en empty field in an bookmark via this code.

[white_flag]

ok ..I insert in the end of the text file "=" without anything after ..and this do the trick. Question: This is correct conform VBA healthy way? (please see attachment) .

[fumei]

I have no idea what you are actually asking.

[white_flag]

I just want to say if this code is good(healthy) from an VBA programmer (point of view) and it is not an mess or an code that is "sick".

thank you :)

[fumei]

Hard to say, as I am very unclear as to what you doing. You do not declare your variables. Obviously you are not using Option Explicit, and I strongly suggest you start to do this.

For example:
Set f = fs.GetFile(ActiveDocument.Path & "\bkmk." & j)
What is the purpose of this?

1. f is never declared
2. f is never used afte rit is Set
3. why are you even using FSO in the first place?

And later...
nEqualsSignPos = InStr(sTemp, "=")




sTemp is never declared
sTemp is not given (at least in this procedure) any value.

So what is going on with:
Input #nFile, sTemp
or...
[vba]

I do not know if it is sick or not. Technically (other than the lack of declaring your variables) there is nothing "wrong", although I have to admit I get a sort of warning bell in my head trying to read it.

[white_flag]

module

Option Explicit
'fumei
Sub FillBM(strBM As String, strText As String)
Dim r As Range
Set r = ActiveDocument.Bookmarks(strBM).Range
r.Text = strText
ActiveDocument.Bookmarks.Add Name:=strBM, Range:=r
End Sub



Option Explicit
Dim data
Dim i As Long, fs, s, ts, f, j, k
Dim InputData, T
Dim nFile As Integer
Dim sTemp As String
Dim nEqualsSignPos As Integer

Private Sub fill()
nFile = FreeFile()
Set fs = CreateObject("Scripting.FileSystemObject")
For j = 1 To 3
If Controls("OptionButton" & j) = True Then
Set f = fs.GetFile(ActiveDocument.Path & "\bkmk." & j)
Open ActiveDocument.Path & "\bkmk." & j For Input As #nFile
Do While Not EOF(nFile)
Input #nFile, sTemp
For i = 1 To 2
If Len(sTemp) > 0 Then
nEqualsSignPos = InStr(sTemp, "=")
nEqualsSignPos = nEqualsSignPos + 1
If InStr(sTemp, Controls("ComboBox" & i).Value) Then
Call FillBM("book" & i, Mid(sTemp, nEqualsSignPos))

End If
End If
Next
Loop
Close nFile
End If
Next
End Sub

Private Sub CommandButton1_Click()
fill
End Sub

Private Sub UserForm_Initialize()
Dim i As Long, fs, s, ts, f
Set fs = CreateObject("Scripting.FileSystemObject")
For j = 1 To 3
If Controls("OptionButton" & j) = True Then
Set f = fs.GetFile(ActiveDocument.Path & "\bkmk." & j)
Set ts = f.OpenAsTextStream(1)
data = Split(ts.Readall, "|")
For i = 1 To 2
Populate i
Next
End If
Next
'Call ShowSomething
End Sub

Sub Populate(i As Long)
Dim InputData, T
InputData = Split(data(i - 1), Chr(13))
For Each T In InputData
If Len(T) > 0 Then _
Me.Controls("ComboBox" & i).AddItem _
Replace(T, Chr(10), "")
Next

End Sub

Sub ShowSomething()
Dim ctl As Control
For Each ctl In Me.Controls
If TypeOf ctl Is MSForms.ComboBox Then
Me.Controls(ctl.Name).ListIndex = 0
End If
Next
End Sub


I forget to put entire code, it was put in attachment .. that was just an piece from the rest ..mia culpa.
I try to separate text(input data) from logical (programming). Like thsi if I want to change something in template I just have to change the text file (I find this solution good for me) .. so an opinion (not a dramatical one) ..just to know if I am doing something correct.

thx

[lucas]

You dim all of these publicly, as variants I might add and then dim some of them again within procedures?
Option Explicit
Dim data
Dim i As Long, fs, s, ts, f, j, k

[white_flag]

correct ..I will put then where they are belong.

[lucas]

fs should be dimmed as an object

f as string. There are more.

Look up variant types in help.

[white_flag]

ok :) thx ..I will look

[fumei]

fs should be Dimmed as a FileSystemObject, not just Object.

ALL variables should be dimmed in the place that they belong, and according to their usage.

Option Explicit
Dim data
Dim i As Long, fs, s, ts, f, j, k
Dim InputData, T

For example, T should NOT be a public variable (nor a Variant). It is used as a string in ONE procedure...therefore it should be declared, as a string in that one procedure.

You should read up on Scope.