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

Add optional tweaking of the .gv output #215

Merged
merged 4 commits into from
Sep 14, 2021
Merged

Conversation

kvid
Copy link
Collaborator

@kvid kvid commented Feb 11, 2021

@tim292stro
Copy link

Will check it this weekend

@tim292stro
Copy link

Man that was a long "this weekend" finally getting to it this afternoon... :-|

Solves wireviz#174 intermediately by allowing low level .gv tweaking.
@kvid kvid marked this pull request as ready for review September 6, 2021 22:57
@formatc1702
Copy link
Collaborator

Could you please provide a sample file where you append and override some stuff in the YAML?
Doesn't have to be part of the PR, a comment here is enough.
Thanks!

docs/syntax.md Outdated Show resolved Hide resolved
if len(value) == 0 or ' ' in value:
value = value.replace('"', r'\"')
value = f'"{value}"'
# TODO?: If value is None: delete attr, and if attr not found: append it?
Copy link
Collaborator

@formatc1702 formatc1702 Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both ideas make sense. I guess this would be a separate PR in the future?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was the plan. The current implementation should be enough to solve the original issue and a lot of similar experimental tweaking, but I don't expect a huge number of users will need such tweaking in their harnesses. Maybe it'll mainly be a tool for developers to test variations of .gv output before suggesting a proper implementation of a new feature.

Copy link
Collaborator Author

@kvid kvid Sep 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just implemented these features and committed them to this PR as well.

docs/syntax.md Outdated Show resolved Hide resolved
@kvid
Copy link
Collaborator Author

kvid commented Sep 8, 2021

Could you please provide a sample file where you append and override some stuff in the YAML?
Doesn't have to be part of the PR, a comment here is enough.
Thanks!

See the already provided link to such an example in the second bullet of this PR description.
Edit: Here is a full copy of the YAML file: HDMI_Sideband_Tap_tweaked.txt

src/wireviz/Harness.py Outdated Show resolved Hide resolved
dot.body[i] = entry

if self.tweak.append is not None:
if isinstance(self.tweak.append, list):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason that the append section should be a list, and not a string?
Is there a benefit to having multiple list entries for different tweaks, as opposed to all of them concatenated into one string? I am not opposed to it, just genuinely curious.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, I used a list of quoted strings to easily specify leading spaces where I wanted them. Leading spaces in multiline strings seems to be stripped by the YAML parser, but maybe you know a way to avoid that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you include the indentation indicator (source), any additional leading spaces are preserved. I quickly tested this and it works. Example:

tweak:
  append: |2  # two leading spaces will be interpreted as indentation; any more will be preserved
              // Tweaking two cable nodes to keep close to each other
              subgraph cluster1 {
                color=yellow // Same as background to hide the surrounding frame
                TPPinkSilver
                TPBlueSilver
              }

kvid and others added 3 commits September 12, 2021 23:26
- Improve syntax description
- Improve typecheck() function parameter names and type hints

Co-authored-by: Daniel Rojas <github@danielrojas.net>
@formatc1702 formatc1702 merged commit db05514 into wireviz:dev Sep 14, 2021
formatc1702 added a commit that referenced this pull request Sep 14, 2021
@formatc1702
Copy link
Collaborator

formatc1702 commented Sep 14, 2021

Thanks. I'll leave #174 open, since it is not directly solved by this. If there is any other issue/PR you think should be included in the changelog, let me know!

formatc1702 added a commit that referenced this pull request Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants