Skip to content

Don't escape special chars when they are in inline_code #743

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

Closed

Conversation

alexjfisher
Copy link
Contributor

Fixes #742

This is Work in Progress. I don't think it's quite right. I think there are at least edge cases where this isn't good enough. (eg an issue/PR title where one of the special chars occurs twice, once inside inline code and again outside)

@alexjfisher alexjfisher changed the title WIP: Don't escape special chars when they are in inline_code Don't escape special chars when they are in inline_code Dec 19, 2019
@skywinder skywinder added the WIP work in progress label Dec 30, 2019
@ferrarimarco
Copy link
Contributor

This is a WIP since months. Shall we close it and move on?

@olleolleolle
Copy link
Collaborator

GitHub now has the option for PR authors to turn them into Draft PRs, to show that they're not looking for code review just yet. Perhaps that something for this PR? Or, it can function as a comment on the registered Issue, even if closed.

@ekohl
Copy link
Contributor

ekohl commented Apr 11, 2020

I'm not sure why @skywinder added WIP since @alexjfisher removed WIP from the title.

@alexjfisher
Copy link
Contributor Author

We've been using this change at Vox Pupuli for the last few months at least. I Initially, I wasn't sure it'd be good enough, but in practice, it's worked really well.

@olleolleolle
Copy link
Collaborator

olleolleolle commented Apr 11, 2020

Can we formulate a test for #encapsulate_string(str) which explains the escaping as a unit test?

It's great to hear that it seems to do the right thing, and thanks for taking the time to follow up.

Example to start from: here I blatantly #send to the private method (which has no instance data dependencies):

# frozen_string_literal: true

module GitHubChangelogGenerator
  RSpec.describe Section do
    let(:options) { { } }
    subject(:section) { described_class.new(options) }

    describe '#encapsulate_string' do
      let(:string) { '' }

      context 'with the empty string' do
        it 'returns the string' do
          expect(section.send(:encapsulate_string, string)).to eq string
        end
      end

      context 'with a string with an escape-needing character in it' do
        let(:string) { '<Inside> and outside' }

        it 'returns the string escaped' do
          expect(section.send(:encapsulate_string, string)).to eq '\\<Inside\\> and outside'
        end
      end
    end
  end
end

@olleolleolle
Copy link
Collaborator

@alexjfisher I have finished writing a tiny unit test for the new kind of input supported that you created. I have kept your commit, and added my own. I'll make a PR to "carry" these commits into master. Hope that's alright!

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Special characters inside inline_code incorrectly escaped
5 participants