Skip to content
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.

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

Select pins in dipchip #550

Merged
merged 11 commits into from Jun 7, 2021
Merged

Select pins in dipchip #550

merged 11 commits into from Jun 7, 2021

Conversation

@Skillmon
Copy link
Contributor

@Skillmon Skillmon commented Jun 6, 2021

This PR will add the key multipoles/draw only pins that takes a comma separated list of numbers that specify which pins will be drawn in dipchip. The pin anchors will be set accordingly. The key takes a special value all that means that every pin is drawn.

@Rmano
Copy link
Collaborator

@Rmano Rmano commented Jun 6, 2021

It seems that it is impossible to add a commit here (a pity, it would be nice to have mixed author PRs...)
I know it's a hack, but this patch:

diff --git a/tex/pgfcircutils.tex b/tex/pgfcircutils.tex
index 54e09b7..c96a382 100644
--- a/tex/pgfcircutils.tex
+++ b/tex/pgfcircutils.tex
@@ -120,6 +120,21 @@
 }
 \long\def\ctikzsubcircuitactivate#1{\csname #1@setanchors\endcsname}
 
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%% some hack to support old TikZ (to remove in the future!)
+\ifcsname pgfutil@unexpanded\endcsname\else
+    \let\pgfutil@unexpanded\unexpanded
+\fi
+\ifcsname pgfutil@ifx\endcsname\else
+    \long\def\pgfutil@ifx#1#2{%
+        \ifx#1#2%
+            \expandafter\pgfutil@firstoftwo
+        \else
+            \expandafter\pgfutil@secondoftwo
+    \fi}
+\fi

Will make Travis happy (I think) and also all the users of TeXLive2019 that use the "last release" from the GitHub page (I know there is quite a number).

@Skillmon
Copy link
Contributor Author

@Skillmon Skillmon commented Jun 6, 2021

There is still no formal test here whether the feature really works or not. All we do know is that there is no real error for the current version of the manual.

So what's still missing here?

  • some documentation using this feature (and documenting it...)
  • more testing, best outside of the manual as well (and using the edge cases inside the manual, so that they will be checked for eternity)

I'm pretty sure the list parsing macros are stable (well, as stable as documented in the sources). Edge cases that will result in a low level TeX error are:

  • If one specifies anything formatted as a num-range with non-numeric data (or anything that's not legal inside \numexpr)
  • lists can't contain explicit \par tokens, this will throw the typical error message

Unsupported input/stuff:

  • numeric data that isn't already expanded (exception is the start and end of a num-range, those will be expanded using \numexpr) will not be expanded
  • lists can't contain explicit tokens of character code 81 and category code 3 (that is an uppercase Q with category math shift) -- this is actively handled, such elements will be discarded from the list and result in an error message
  • lists can't contain the token \pgf@circ@set@list (as that is used as the end-of-list-marker during parsing) -- this is not actively handled and will result in undefined behaviour

Everything else should work.


I'm entirely unsure on the PGF side of this PR, as I have not much experience with low level PGF, so I'm quite unsure whether everything works. Some quick small tests indicated only expected behaviour, but that doesn't mean I'm confident with this stuff.

@Rmano
Copy link
Collaborator

@Rmano Rmano commented Jun 6, 2021

I think that it looks wonderful. I checked with a few extreme cases (empty list, out-of-range pins) and it works.

Do you want to add the doc or I do that?

Also, I like the names (draw only pins). I would use the same key for the qfpchip (in the future, when extending it to them) and
draw only left/top/bottom/right pins for muxdemuxes. That will make the last ones a killer configurable object...

@Skillmon
Copy link
Contributor Author

@Skillmon Skillmon commented Jun 6, 2021

draw only pins was your idea :) I'm currently at a short documentation.

@Skillmon
Copy link
Contributor Author

@Skillmon Skillmon commented Jun 6, 2021

Before I forget. I saw there is some change-log in place. I haven't touched that :)

@Rmano
Copy link
Collaborator

@Rmano Rmano commented Jun 6, 2021

Ah yes, the file is the Changelog.md in the top dir, in markdown format. Normally I add attribution there also, look at the previous entries. Do not forget to add some attribution in the code also if you like!

@Rmano
Copy link
Collaborator

@Rmano Rmano commented Jun 7, 2021

Hi @Skillmon --- I was thinking that I'll probably squash and merge this, to avoid the "red cross" points in history that can hinder future bisections. I will wait till you add changelog and attribution (unless you tell me you don't want those).

@Skillmon
Copy link
Contributor Author

@Skillmon Skillmon commented Jun 7, 2021

@Rmano Added some attribution notices. If the plan is to also provide this for qfpchip it might make sense to delay the changelog entry until after that is done and only add one entry for both.

Should be ready to merge now.

@Rmano
Copy link
Collaborator

@Rmano Rmano commented Jun 7, 2021

If the plan is to also provide this for qfpchip it might make sense to delay the changelog entry until after that is done and only add one entry for both.

Ok (I normally add the changelog line and then I edit it with the next commit, but it's more or less the same --- I do not think that inter-releases things have much visibility anyhow)

@Rmano Rmano merged commit 1630d56 into circuitikz:master Jun 7, 2021
1 check passed
@Rmano
Copy link
Collaborator

@Rmano Rmano commented Jun 7, 2021

I will start to work on porting this to qfpchips and later muxdemux tomorrow or later... I want to really understand the code before. I think I got most of it, but well... let's see. I hope you do not mind if I rename a couple macros here.

@Skillmon
Copy link
Contributor Author

@Skillmon Skillmon commented Jun 7, 2021

Yes, sure you might rename it. In the end it should be readable to the maintainers, not to an occasional contributor :) If you have any other questions regarding the code, feel free to ask.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants