AutoLisp Feedback

[ArchD]

I have this lisp that I've created but was wondering if someone could look it over and give me some feedback. 

I would like to move away from using (command "") in side of it but I'm am just now picking up AutoLisp and I figure your feedback would help me learn ActiveX.

Basically what this lisp does is checks the drawing for color based pen set or style based, and then creates a layer with all of its properties. Once created it creates a linear dimension. After all is done it reverts back to previous layers and settings. 

It also has an error handler in there that I think is cumbersome.

Not to mention that I can't seem to be able to suppress the "undo" "m" part of it.

So if anyone is willing to give me feedback or help me convert anything to ActiveX, I would really appreciate your time.

Thanks!

(defun C:CODLinear (/ CurCMDEcho curNOMUTT CurLay CurPSM)
	(command "undo" "m")
	(setq CurCMDEcho (getvar "CMDECHO"))
	(setq CurNOMUTT (getvar "NOMUTT"))
	(setq CurPSM (getvar "PSTYLEMODE"))
	(setvar "CMDECHO" 0)
	(setvar "NOMUTT" 1)
	(command "undo" "m")
	
    (defun *error* ( msg )
		(setvar "CMDECHO" 0)
        (if (not (member msg '("Function cancelled" "quit / exit abort")))
            (princ (strcat "\nError: " msg))
        )
		(progn
			(command-s "undo" "b")
			(setvar "CMDECHO" CurCMDEcho)
			(setvar "NOMUTT" CurNOMUTT)
		)
		(princ)
    )

	(progn
		(if (= CurPSM 1)
		
			(progn
				(setq CurLay (getvar "clayer"))
				(command)
				(command "-LAYER" "M" "P-DIM-COD CENTER" "C" "2" "" "")
			)
			
			(progn
				(setq CurLay (getvar "clayer"))
				(command)
				(command "-LAYER" "M" "P-DIM-COD CENTER" "C" "2" "" "PS" "2 - Very Thin" "" "")
			)
			
		)
		(setvar "CMDECHO" 1)
		(command "_dimlinear" (while (= 1 (getvar "cmdactive")) (command pause)))
		(setvar "CMDECHO" 0)
		(setvar "clayer" CurLay)
		
	)
	(setvar "CMDECHO" CurCMDEcho)
	(setvar "NOMUTT" CurNOMUTT)
	(princ)
)

[Kent1Cooper]

That *error* handler looks perfectly normal except for having a (progn) wrapper around parts of it that isn't necessary.  Likewise, there's another (progn) you don't need in the main part.  The (progn) function serves to "collect" multiple operations into one function, and is typically used when you need multiple operations to be, collectively, the 'then' expression or the 'else' expression for an (if) function, since those must be single expressions.  You don't have that situation here, if you also pull the CurLay setting out of the (if) function arguments [since it's happening in either case]..

But you do need to add *error* to the localized variables list, so it will revert to AutoCAD's standard one afterwards.

I don't think you need those (command) functions without arguments.  That serves to cancel a command that may already be in progress, but you're not going to be able to type in a  (defun C:  -variety command name except at the Command prompt anyway, in which case no other command will be active.

In this case, it looks like you could use Undo Begin and End, rather than Mark [which is in there twice] and Back, in which case you can do the begin/end "silently" with (vla-) functions, and not need to suppress that, and Undo any partial operation that may have happened with a simple U command, which can still be within command-echo suppression.

And I think you shouldn't need to do the Nomutt thing.

You need to end  the (command) function that starts DIMLINEAR, leaving you in that command, before the (while) thing for continuing to take User input until the command is completed.  And I changed the  (= 1  test for the CMDACTIVE System Variable to  (< 0  instead -- the 1 bit will be in  that value while the command is operating, but it can potentially get other bits in it, so what you want to check for is that it's anything more than 0.

Try something like this [untested] :

(defun C:CODLinear (/ *error* doc CurCMDEcho CurLay CurPSM)

  (defun *error* ( msg )
    (if (not (member msg '("Function cancelled" "quit / exit abort")))
      (princ (strcat "\nError: " msg))
    )
    (vla-endundomark doc)
    (command-s "_.u")
    (setvar "CMDECHO" CurCMDEcho)
  )
	
  (vla-startundomark (setq doc (vla-get-activedocument (vlax-get-acad-object))))
  (setq
    CurCMDEcho (getvar "CMDECHO")
    CurPSM (getvar "PSTYLEMODE")
    CurLay (getvar "clayer")
  )
  (setvar "CMDECHO" 0) 
  (if (= CurPSM 1)
    (command "-LAYER" "M" "P-DIM-COD CENTER" "C" "2" "" ""); then
    (command "-LAYER" "M" "P-DIM-COD CENTER" "C" "2" "" "PS" "2 - Very Thin" "" ""); else
  )
  (setvar "CMDECHO" 1)
  (command "_dimlinear")
  (while (< 0 (getvar "cmdactive")) (command pause))
  (setvar "clayer" CurLay)
  (setvar "CMDECHO" CurCMDEcho)
  (vla-endundomark doc)
  (princ)
)
(vl-load-com) ; just in case

Come to think of it, you could replace the duplication of this part:

  (if (= CurPSM 1)
    (command "-LAYER" "M" "P-DIM-COD CENTER" "C" "2" "" ""); then
    (command "-LAYER" "M" "P-DIM-COD CENTER" "C" "2" "" "PS" "2 - Very Thin" "" ""); else
  )

with this:

  (command "-LAYER" "M" "P-DIM-COD CENTER" "C" "2" ""); spelled out only once
  (if (= CurPSM 1) (command "") (command "PS" "2 - Very Thin" "" ""))

[Ranjit_Singh]

It looks OK, except for few unnecessary progns. For creating layer, maybe do not use activex. You need to retrieve the layer collection, add the new layer and then assign properties. I find command much easier. Other option is to use dxf codes to create the entity and assign needed properties in there.

(defun c:codlinear  (/ adoc *error* vars)
 (defun *error*  (msg)
  (mapcar 'setvar '(cmdecho nomutt clayer) vars)
  (if (not (member msg '("Function cancelled" "quit / exit abort" "ok")))
   (princ (strcat "\nError: " msg)))
  (vla-endundomark adoc)
  (princ))
 (vla-startundomark (setq adoc (vla-get-activedocument (vlax-get-acad-object))))
 (setq vars (mapcar 'getvar '(cmdecho nomutt clayer)))
 (setvar 'cmdecho 0)
 (setvar 'nomutt 1)
 (entmake '((0 . "LAYER") (100 . "AcDbSymbolTableRecord") (100 . "AcDbLayerTableRecord") (2 . "P-DIM-COD CENTER") (70 . 0) (62 . 2)))
 (if (zerop (getvar 'pstylemode))
  (command "PS" "2 - Very Thin" "" ""))
 (setvar 'cmdecho 1)
 (command "_dimlinear" (while (< 0 (getvar 'cmdactive)) (command pause)))
 (*error* "ok"))

[john.uhden]

That was rather well demonstrated.  But I am wondering, rather than...

(defun *error* ( msg )
    (if (not (member msg '("Function cancelled" "quit / exit abort")))
      (princ (strcat "\nError: " msg))
    )
    (vla-endundomark doc)
    (command-s "_.u")
    (setvar "CMDECHO" CurCMDEcho)
  )

It might be better to do what some sage scholar here taught us years ago... to use the localized *error* function to clean up a normal exit...

(defun *error* ( msg )
  (cond
    ((not msg))
    ((wcmatch (strcase msg) "*QUIT*,*CANCEL*,*EXIT*"))
    (T (princ (strcat "\nError: " msg))
      (command-s "_.u")
    )
  )
  (setvar "CMDECHO" CurCMDEcho)
  (vla-endundomark doc)
  (princ)
)

That way the AutoLisp command can end with (*error* nil) and do the cleanup instead of just (princ).

I am concerned about the (command-s "_.u").  Should that be invoked only if there is an error, or only if it is cancelled, or maybe only if some particular action were taken in the main function, or a combination of things, or just no matter what?

If I am being a PITA, please just tell me and I will go away.

[ArchD]

Wow, really great feedback. Thanks guys. Trying to learn this stuff and it seems I can get things to work, but I knew it was done a little wonky. Taking the weekend off then I'll make the modifications.

Thanks so much, very helpful indeed.

[Kent1Cooper]

---

@Ranjit_Singh wrote:

....

...
 (entmake '((0 . "LAYER") (100 . "AcDbSymbolTableRecord") (100 . "AcDbLayerTableRecord") (2 . "P-DIM-COD CENTER") (70 . 0) (62 . 2)))
 (if (zerop (getvar 'pstylemode))
  (command "PS" "2 - Very Thin" "" ""))....

---

If you're going to use (entmake) to create the Layer, then you will not be in a Layer command, and that last line will have to get you into one, before you start using Layer-command options, i.e.:

(command "_.layer" "PS" ....

[Kent1Cooper]

---

@john.uhden wrote:

.... 

(defun *error* ( msg )
  (cond
    ((not msg))
    ((wcmatch (strcase msg) "*QUIT*,*CANCEL*,*EXIT*"))
    (T (princ (strcat "\nError: " msg))
      (command-s "_.u")
    )
  )
  (setvar "CMDECHO" CurCMDEcho)
  (vla-endundomark doc)....

....

---

That won't work right, because if something has been partially  done before an error occurs, that you want to undo, the Undo End part has to come before  the U command.

[john.uhden]

But if the startundomark were set before anything else happens, then everything between startundomark and endundomark is encapsulated.  Or does (command "_.U") undo everything back to the startundomark?  I don't think so.  I believe it undoes only one command.

Ah, but what I think you are saying is that if something within the function errors out, then you want everything to be undone.  Okay, that makes sense.  That's why I was asking where to put the (command "_.U").  I guess I happened to put it in the right spot.

[marko_ribar]

Haven't read all things, but to reply to last post :

If you want to undo thing to (vla-startundomark adoc), then use :

(command-s "_.UNDO" "_B")

If you want to undo just single command - (vla- or ALISP functions won't count), then use :

(command-s "_.UNDO" "1"), which is IMO the same as (command-s "_.U"), but I prefer first one - it's just me... 🙂

[john.uhden]

Good cal!!  Since I think there is more than one command in the body of the OP's code, then an Undo Back would be better to use.

I'll have to test (because I don't know for sure) if a vla-startundomark is the same as a (command "Undo" "M").

If it's not the same, and NOMUTT is turned on, then it could undo everything back to the beginning of the session <ouch>.

I certainly don't want to be blamed for that.

[ActivistInvestor]

---

@john.uhden wrote:

But if the startundomark were set before anything else happens, then everything between startundomark and endundomark is encapsulated.  Or does (command "_.U") undo everything back to the startundomark?  I don't think so.  I believe it undoes only one command.

Ah, but what I think you are saying is that if something within the function errors out, then you want everything to be undone.  Okay, that makes sense.  That's why I was asking where to put the (command "_.U").  I guess I happened to put it in the right spot.

---

The names of the APIs are confusing.

StartUndoMark() and EndUndoMark() are just wrappers for (command "._UNDO" "_Begin") and (command "._UNDO" "_End") respectively, so a single (command "._U") will undo the entire group that was just ended via EndUndoMark().

[ActivistInvestor]

---

@marko_ribar wrote:

Haven't read all things, but to reply to last post :

If you want to undo thing to (vla-startundomark adoc), then use :

(command-s "_.UNDO" "_B")

If you want to undo just single command - (vla- or ALISP functions won't count), then use :

(command-s "_.UNDO" "1"), which is IMO the same as (command-s "_.U"), but I prefer first one - it's just me... 🙂

---

See my other reply. 

I would strongly advise against the use of the UNDO/Back subcommand, because it presumes that a mark had been set, which may not always be the case, depending on the scope of the *error* function's use.

Many years ago, thousands of users were plagued by a mysterious and particularly nasty bug, which basically was a result of the old BONUS tools error handler being left as the global *error* function. That *error* handler used (command "._UNDO" "Back"), and when it kicked-in when there was no previous UNDO Mark used, guess what happened?  The entire editing session was undone, and could not be redone.

[john.uhden]

It's so easy to see when somebody turns on a light.

[Kent1Cooper]

---

@john.uhden wrote:

But if the startundomark were set before anything else happens, then everything between startundomark and endundomark is encapsulated.  Or does (command "_.U") undo everything back to the startundomark?  I don't think so.  I believe it undoes only one command. ....

---

That is correct, if no endundomark has been set.  But if you put a endundomark [the same as Undo End], then using the U command, whether in an *error* handler or outside the routine after it's done, the entire set of things thus encapsulated  is all undone together, because the Begin/End wrapper makes it all collectively one  thing from the U command's point of view -- I've done it in many routines.  If you put a U command in without  having put in an endundomark, then it will undo only the last individual command, not back to the start.

I've even combined  the two -- an Undo Begin at the beginning of the routine, and within it, Undo Mark and Undo Back settings to serve an internal Undo option  in the command, and an Undo End at the end [and in the *error* handler].  That way, the Undo option  when called from within  the command undoes only the latest iteration, and you can go on to do more of whatever it does, but if you finish and then do U outside  it, all iterations that it did are undone collectively.  [An example: LabelTextTrim.lsp, available here.]

[john.uhden]

In spite of your verbosity, I appreciate the wisdom you share.  Thank you, Kent.

[ArchD]

Ranjit, this is the sort of code I'm looking for. Unfortunately it doesn't seem to work. In color based pen sets it doesn't create the layer, in style based it breaks do to the command line that Kent1Cooper pointed out.

I'm thinking that it should go something more like this?

(if
     (zerop (getvar 'pstylemode))
          (entmake '((0 . "LAYER") (100 . "AcDbSymbolTableRecord") (100 . "AcDbLayerTableRecord") (2 . "P-DIM-COD CENTER") (70 . 0) (62 . 2)))
	  (entmake '((0 . "LAYER") (100 . "AcDbSymbolTableRecord") (100 . "AcDbLayerTableRecord") (2 . "P-DIM-COD CENTER") (70 . 0) (62 . 2) (380 . "2 - Very Thin"))))

I'm looking at the DXF codes and believe that (380 . "2 - Very Thin") should do for adding the plot style but unfortunately I can't get the code to work even without the if statement. 

Thanks for all the input from everyone. Finally starting to get the thought process behind the error functions and DXF group codes. I still have a long way to go though.

[Ranjit_Singh]

I cannot test it since I am not able to get the PS command in the -layer options. I believe it only shows up when pstylemode is 0. Can you post a sample dwg with some sample data, layers etc. and the correct stb file.

[ArchD]

I've attached a dwg using stb style pen set with a pen set. 

In the drawing there is a layer named P-TEXT set to yellow with the plot style set to "2 - Very Thin". When the command is executed it would create the layer and set it to current prior to using the dimlinear command.

If the layer is present just use that one instead of creating a new one.

Thanks again for your help.

[Ranjit_Singh · Accepted solution]

OK. Try this

(defun c:codlinear  (/ adoc *error* vars)
  (defun *error*  (msg)
    (mapcar 'setvar '(cmdecho nomutt clayer) vars)
    (if (not (member msg '("Function cancelled" "quit / exit abort" "ok")))
      (princ (strcat "\nError: " msg)))
    (vla-endundomark adoc)
    (princ))
  (vla-startundomark (setq adoc (vla-get-activedocument (vlax-get-acad-object))))
  (setq vars (mapcar 'getvar '(cmdecho nomutt clayer)))
  (setvar 'cmdecho 0)
  (setvar 'nomutt 1)
  (if (null (tblsearch "layer" "P-DIM-COD CENTER"))
    (entmake (append '((0 . "LAYER") (100 . "AcDbSymbolTableRecord") (100 . "AcDbLayerTableRecord") (2 . "P-DIM-COD CENTER") (70 . 0) (62 . 2))
                     (if (zerop (getvar 'pstylemode))
                       (list (cons 390 (psname "2 - Very Thin")))))))
  (setvar 'clayer "P-DIM-COD CENTER")
  (setvar 'cmdecho 1)
  (command "_dimlinear" (while (< 0 (getvar 'cmdactive)) (command pause)))
  (*error* "ok"))

(defun psname  (x)
  (cdr (assoc -1 (dictsearch (cdr (assoc -1 (dictsearch (namedobjdict) "ACAD_PLOTSTYLENAME"))) x))))

[ArchD]

Thank you sir! This is perfect, much appreciated.