erkin / ponysay

Pony rewrite of cowsay.
GNU General Public License v3.0
1.22k stars 82 forks source link

Fix visuals, --restrict #313

Open Tonyl314 opened 1 year ago

Tonyl314 commented 1 year ago

I have fixed three things:

Default ANSI code Ponysay was using the "21" ANSI code to reset boldness; however, this functionality does not seem to be widely supported (see this gist or this answer). On my terminal, for example, it serves as double-underline, leading to a very ugly help message. For this reason, I have replaced the code with a "0", as that is a universal reset.

Fix --restrict There was an unmatched bracket in a pony's metadata which led to the "metadata" file not being created on setup. This fixes issue #266, making the --restrict option work again.

Backslash escaping There was an instance of a docstring using unescaped backslashes, which in turn threw Python warnings.

JotaRandom commented 1 year ago

I will ask for anyother that want to comment to comment, but i see no issue if this is more compliant to ANSI than what we have

So the only thing remaining is updating the CHANGELOG and related stuff (changelog from the manuals/ponysay.texinfo aroung line 3413)

Tonyl314 commented 1 year ago

@JotaRandom Should I update it? Which of the fixes should appear there?

maclermo commented 11 months ago

I don't agree with the fix. You should use multiline strings with the r prefix (raw):

r"""
@param link:str The \-directional balloon line character
"""

That way the docstring stays intact.

teferi commented 7 months ago

@Tonyl314 Can I ask you to look at @maclermo suggestion and amend the CHANGELOG as @JotaRandom requested. The syntax warning is really annoying on 3.12 python

Tonyl314 commented 7 months ago

I switched to using a raw docstring as @maclermo suggested and updated the changelog. Is there anything else left to do?

codegod100 commented 3 months ago

merge?

SamuelCano03 commented 3 months ago

still seeing the invalid sequence warning. ponysay 3.0.3 python 3.12.4

  1. should i downgrade python?
  2. I dont want to downgrade python.

the warning appears for every ponysay option

Tonyl314 commented 3 months ago

@SamuelCano03 Are you getting the exact same warning as in #314? Does it also appear when you run src/balloon.py directly?

SamuelCano03 commented 3 months ago

@SamuelCano03 Are you getting the exact same warning as in #314? Does it also appear when you run src/balloon.py directly?

Tonyl314 commented 3 months ago

@SamuelCano03 Oh, then you don't have my fork. The fix hasn't been merged yet, so it's only available here (needs to be built from source).

SamuelCano03 commented 3 months ago

@Tonyl314 sorry for silly question. Does it mean that at the moment the solution is installing from source from your fork? later will be available on pacman/apt repositories?

Tonyl314 commented 3 months ago

@SamuelCano03 Yup! You would need to uninstall the Arch package, download the source code from the fork and run setup.py. Hopefully someone can merge this soon so that the fix is easier to access…

codegod100 commented 3 months ago

sudo merge