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

spdx3: element_writer: switch from tab characters to two spaces #785

Merged
merged 3 commits into from
Aug 9, 2024

Conversation

stanislaw
Copy link
Contributor

Hello everyone,

This is my first issue to this repository. I am new to SPDX and apologize for possible misunderstanding of the intentions behind the code I am modifying.

I am integrating the SPDX 3.0 export to the StrictDoc tool, see strictdoc-project/strictdoc#1541.

The starting point for this issue is the fact that the Creation Information block is suddenly nested with a tab character when I try to populate an spdx3.*.SpdxDocument and write to a string buffer. My IDE is set up to ignore tab characters, so the output is displayed as if the indentation level was 1 character wide.

Bildschirmfoto 2024-01-09 um 21 07 31

In this MR, the creation info is unindented, and a test is added to demonstrate the behavior I would expect to see.

@stanislaw stanislaw force-pushed the stanislaw/indentation branch 2 times, most recently from cc2943d to a61bb33 Compare January 9, 2024 20:22
@stanislaw stanislaw force-pushed the stanislaw/indentation branch from a61bb33 to 8452aef Compare January 18, 2024 21:04
@maxhbr
Copy link
Member

maxhbr commented Jan 19, 2024

I think that the indentation was intentional. As sub-structures within an object are also come with indentation in other places. Maybe we could indent with spaces instead of \t to be more stable.

@stanislaw
Copy link
Contributor Author

I think that the indentation was intentional. As sub-structures within an object are also come with indentation in other places. Maybe we could indent with spaces instead of \t to be more stable.

This makes sense. Definitely it would make sense to replace \t with \n\n for example.

An extra thought regarding the indentation of creation info: in YAML things are indented when there is a key that bundles together indented content. On the screenshot above, the role of a "key" is played by a comment Creation Information which to me looks confusing visually because it is just a comment and on actual key.

In YAML this would look like:

creation_info:
  spec_version: ...
  ...

I am ready to both engage or disengage, so how could we proceed? Is it worth to use this MR for replacing \t with \n\n everywhere?

@maxhbr
Copy link
Member

maxhbr commented Jan 19, 2024

we would appreciate if you would make the changes to spaces. But you could also just put it into a new issue, if you want.

Signed-off-by: Stanislav Pankevich <s.pankevich@gmail.com>
@stanislaw stanislaw force-pushed the stanislaw/indentation branch from 8452aef to 221483c Compare January 21, 2024 20:40
@stanislaw
Copy link
Contributor Author

we would appreciate if you would make the changes to spaces. But you could also just put it into a new issue, if you want.

Please check the change I've made.

Copy link
Member

@maxhbr maxhbr left a comment

Choose a reason for hiding this comment

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

LGTM

@stanislaw stanislaw changed the title spdx3: element_writer: unindent creation information spdx3: element_writer: switch from tab characters to two spaces Jan 22, 2024
@armintaenzertng armintaenzertng merged commit 40331ac into spdx:main Aug 9, 2024
30 checks passed
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