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

Fix BOM portion of generated HTML files #95

Merged
merged 14 commits into from
Jul 20, 2020
Merged

Fix BOM portion of generated HTML files #95

merged 14 commits into from
Jul 20, 2020

Conversation

slightlynybbled
Copy link
Contributor

No description provided.

@slightlynybbled slightlynybbled changed the title Fix UTF-8 Encoding error in BOM portion of generated HTML files. Fix BOM portion of generated HTML files Jul 16, 2020
@slightlynybbled
Copy link
Contributor Author

Also fixes BOM display as noted in #86.

src/wireviz/Harness.py Outdated Show resolved Hide resolved
src/wireviz/Harness.py Outdated Show resolved Hide resolved
@slightlynybbled
Copy link
Contributor Author

slightlynybbled commented Jul 17, 2020

The extra slash is an escape character in python strings. Two slashes are required to represent one slash. Actual string being checked is ‘\n’

@kvid
Copy link
Collaborator

kvid commented Jul 17, 2020

The extra slash is an escape character in python strings. Two slashes are required to represent one slash. Actual string being checked is ‘\n’

I know that. :-) When I write '\\n' in code formatting, I mean a two-character string: backslash followed by the letter n. The user might put that in the input text as an attempt to obtain a line break. Currently, this only works for connector attributes in the diagram, and with your commit, also in the HTML BOM, but all other output show it literally as backslash followed by the letter n.

As @formatc1702 describes in his issue #86 recommendation, using YAML multiline strings is a better approach to obtain line breaks, that insert actual newline characters, and that converting '\\n' to newline is then unwanted:

Looking at the Flow Scalars section on yaml-multiline.info, it seems that not converting \n into a newline for plain strings is the expected behavior.

Actual newline characters in the input text is currently supported for all output (either converted to a space or to a proper line break for each output).

@formatc1702
Copy link
Collaborator

formatc1702 commented Jul 17, 2020

If we should not support '\\n' in the input text, then we let the user easily discover his mistake, not hide it in some output.

I agree with @kvid on this one; if the YAML parser does not generate newlines with \n, why should we?
As described in #88, if (for whatever reason) the user wants to use \n, it needs to be within a double quoted string and thus, again, the YAML parser itself will take care of it.

No need for an extra check here!

  • who thought that a four line PR would spark such a discussion :D

@slightlynybbled
Copy link
Contributor Author

I will make the change b/c this is not my package, but I disagree.

  • The \n character has been a new line for most of modern computer history. It is "the way" to create a line break and, if it can be easily supported, it should be.
  • Many/Most users are only using YAML b/c it is the author's choice and are likely not as familiar with YAML syntax.
  • There is currently no documentation for this package to guide the user, so the package should be forgiving.

@formatc1702
Copy link
Collaborator

Thanks.

I will make the change b/c this is not my package, but I disagree.

  • The \n character has been a new line for most of modern computer history. It is "the way" to create a line break and, if it can be easily supported, it should be.

It can be easily supported by adding the double quotes.

  • Many/Most users are only using YAML b/c it is the author's choice and are likely not as familiar with YAML syntax.
  • There is currently no documentation for this package to guide the user, so the package should be forgiving.

I agree on the last point. IMHO, this is an argument for better documentation (which we absolutely need to do), but not to deviate from the clearly defined YAML syntax.

It may seem like a far-fetched corner case, but an example that comes to mind is somebody using a Windows path in the notes field C:\Documents\new harness\file.ext and wondering why it's split.

@formatc1702 formatc1702 merged commit a418005 into wireviz:dev Jul 20, 2020
@kvid
Copy link
Collaborator

kvid commented Jul 20, 2020

Edit: This PR was merged in while I was writing my comment, but my comment is still valid. I hope @slightlynybbled is notified about my comment even if the PR was closed when merged in.

@slightlynybbled I respect your opinion, and I fully agree that not everyone has full knowledge about YAML syntax (including myself). Please create a new issue where you suggest how to guide the user about line breaks and similar issues. Maybe some additions to README, tutorials, and/or examples? You'll find a lot of examples on yaml-multiline.info - please create a few examples for this project based on what you think the users might need, e.g. something like this:

connectors:
  X1:
    pincount: 4
    notes: |
        First line
        Second line
  X2:
    pincount: 4
    notes: "First line\nSecond line"

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