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

Revert \(bu to \[ci] change #51

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented Jul 15, 2020

This reverts commit e7df1f0, reversing changes made to c3fd67e.

Commit was introduced as a aesthetic change when using -Kutf8 -Tutf8 in rtomayko/ronn#71, which closed ronn issue rtomayko/ronn#70.

Quoting from the issue that led to the PR:

This may seem a little strange, it's more aesthetic then anything
else. When troff is run with -Kutf8 -Tutf8 to ensure that utf8
characters appear correctly then the bullets generated by (bu appear as
small squares (I'm using 13pt Pragmata Pro).

However, in bundler we're not using -Tutf8, but -Tascii, and this results in weird characters for list items as reported here.

Since this change never made it into any ronn release, I think reverting it could encourage adoption of ronn-ng as a drop-in replacement of ronn. At least for us :)

@apjanke
Copy link
Owner

apjanke commented Jul 15, 2020

Based on reading this PR and the linked upstream issues, I think your change here is probably the right thing to do. But I'm going to have to do a little reading on troff's handling of UTF8 vs. ASCII and the bullet markers before I'll be comfortable merging it.

How urgent is this for you? Unfortunately I have a lot on my plate at the day job this week.

@apjanke apjanke added the bug Something isn't working label Jul 15, 2020
@deivid-rodriguez
Copy link
Contributor Author

Not urgent at all, take your time :)

This reverts commit e7df1f0, reversing
changes made to c3fd67e.

Commit was introduce as a aesthetic change when using `-Kutf8 -Tutf8`.

Quoting from the issue that led to the PR:

> This may seem a little strange, it's more aesthetic then anything
> else. When troff is run with -Kutf8 -Tutf8 to ensure that utf8
> characters appear correctly then the bullets generated by \(bu appear as
> small squares (I'm using 13pt Pragmata Pro).

However, in `bundler` we're not using `-Tutf8`, but `-Tascii`, and this
results in weird characters for list items as reported [here](cli-kit/cli-command#78).

Since this change never made it into any `ronn` release, I think
reverting it could encourage adoption of `ronn-ng` as a drop-in
replacement of `ronn`. At least for us :)
@deivid-rodriguez
Copy link
Contributor Author

I rebased this PR for you so it's ready to merge once you review it.

@apjanke
Copy link
Owner

apjanke commented Jul 15, 2020

Thank you!

@apjanke
Copy link
Owner

apjanke commented Jul 16, 2020

Can you give me the TL;DR on how to run ronn and troff on your .ronn file to reproduce your issue with the bullets, so I can see it in action myself?

@apjanke
Copy link
Owner

apjanke commented Jul 16, 2020

Note to self:

References:

@apjanke
Copy link
Owner

apjanke commented Jul 16, 2020

Okay, I've read through all the linked pages and the groff/troff man pages enough to understand what's going on here. I don't think upstream's change of \[bu] to \[ci] was the right thing to do in the first place; that sounds to me more like a user unhappy with their font and terminal configuration, and not an issue with ronn itself. I think "bullet" is a more appropriate character for bulleted lists than "circle".

The fact that the \[ci] change never made it in to a versioned Ronn release gives me a strong presumption in favor of reverting that change in Ronn-NG: It is my intention that Ronn-NG serve as an easy drop-in replacement for the original Ronn. And especially so when I have an actual user complaint about it.

Merging. This will go out in the Ronn-NG 0.10.0 release, which will probably happen in the next couple weeks.

If you encounter any other issues using Ronn-NG as a drop-in replacement for classic Ronn, feel free to let me know, and I'll address those too.

@apjanke apjanke merged commit d74a8c2 into apjanke:master Jul 16, 2020
@deivid-rodriguez deivid-rodriguez deleted the revert_bu_to_ci_change branch July 16, 2020 07:51
@deivid-rodriguez
Copy link
Contributor Author

Thank you!

@deivid-rodriguez
Copy link
Contributor Author

@apjanke First, sorry I never replied to this:

Can you give me the TL;DR on how to run ronn and troff on your .ronn file to reproduce your issue with the bullets, so I can see it in action myself?

To try this against bundler you need to:

Then the man pages in the man/ folder should be built using ronn-ng.

Note that the differences created by ronn-ng in the docs are already committed to the branch, including the \[ci] vs \(bu discrepancy.

@deivid-rodriguez
Copy link
Contributor Author

Second, any news on the 0.10.0 release?

@apjanke
Copy link
Owner

apjanke commented Sep 2, 2020

Second, any news on the 0.10.0 release?

Work has let up a bit now, so I should be able to pull in all the outstanding PRs and assemble an 0.10.0 release over this weekend; figure on seeing it Monday.

@deivid-rodriguez
Copy link
Contributor Author

Thanks a lot @apjanke!

@apjanke
Copy link
Owner

apjanke commented Sep 12, 2020

Work got me after all last weekend. This weekend...

@deivid-rodriguez
Copy link
Contributor Author

Just checking in here in case there's any movement?

@apjanke
Copy link
Owner

apjanke commented Oct 23, 2020

Sorry for the delay on this!

I'm calling the current state of master as ready for the 0.10.0 release. I have tagged a 0.10.0-beta1 release which includes this fix, and published it to RubyGems as version 0.10.0-SNAPSHOT. Are you up for giving this a test? If no issues are found, I will release this as version 0.10.0 tomorrow.

@deivid-rodriguez
Copy link
Contributor Author

Thanks, let me try this now!

@deivid-rodriguez
Copy link
Contributor Author

Everything seems almost ready now for us to migrate. All changes are avoid some unnecessary blank lines and some unnecessary escaping which I don't think cause any changes in how the final output renders. See rubygems/rubygems@b275f4e.

Only issue is in bundle config SYNOPSIS, where we get the following changes:

-\fBbundle config\fR [list|get|set|unset] [\fIname\fR [\fIvalue\fR]]
-.
+.TS
+allbox;
+\fBbundle config\fR [list	get	set	unset] [\fIname\fR [\fIvalue\fR]]
+.TE

which make the synopsis display as empty.

@apjanke
Copy link
Owner

apjanke commented Oct 23, 2020

Ack. That's bad enough it should be fixed before pushing out a release. I'll look in to it.

@simi
Copy link

simi commented Sep 25, 2023

@apjanke any plans for new stable release? Anything I can help with?

@apjanke
Copy link
Owner

apjanke commented Oct 12, 2023

Hi, @simi,

Yeah, I think I've got a set of changes ready to go out for an 0.10 release, which I think. will address this issue? Details and "dev journal" are in #103. Work is on the Let's go, Pikachu! 0.10 branch.

Basic problem here is that I have no idea what the fuck I'm doing here. I don't really know what the Ruby 2.x to 3.x transition means, especially in terms of how downstream packagers consume Ronn-NG or express dependencies, and I'm having trouble just getting it built and running on my dev boxes.

Any input or advice you have here is welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants